41

Is there a better way to write this code in python?

result = slow_function()
if result:
    return result
[...]

The function slow_function can return a value or None and it's slow, so this is not feasible:

if slow_function():
    return slow_function()

There is nothing wrong with the first way, but using a temporary variable seems overkill for python.

This code is pretty useful when you are solving a problem using recursive calls over f and with local assumption, for example you select an item from a list and then check if there is a feasible solution, otherwise you have to choose another one. Something like:

def f(n):
    for x in xrange(n):
        result = slow_function(x):
        if result:
            return result
        [...]

Wouldn't it be better something more idiomatic like:

def f(n):
    for x in xrange(n):
        return slow_function(x) if is not None

This can be extended to check any kind of value. It would be an easy-to-read return if statement.

Additional example for code lovers

Imagine you have a list of lists of numbers:

lists = [[1,2,3],[4,5],[6,7,8],[9,10],...]

and you want to select one item for each list such that there is at most one even number in the selection. There could be a lot of lists, so trying each combination would be wasteful since you can already tell that if you start selecting [1,2,4,...] there could be no feasible solutions.

def check(selected):
    even_numbers = filter(lambda n: (n % 2) == 0, selected)
    return len(even_numbers) < 2

def f(lists, selected=[]):
    if not lists:
        return selected

    for n in lists[0]:
        if check(selected + [n]):
            result = f(lists[1:], selected + [n])
            if result:
                return result

Wouldn't it be better a syntax like:

def f(lists, selected=[]):
    return selected if not lists
    for n in lists[0]:
        if check(selected + [n]):
            return f(lists[1:], selected + [n]) if is not None

The best I've done so far is to turn the function in a generator of feasible solutions:

def f(lists, selected=[]):
    if not lists:
        yield selected
    else:
        for n in lists[0]:
            if check(selected + [n]):
                for solution in f(lists[1:], selected + [n]):
                    yield solution
Improve this question
11
  • 5
    Why is it overkill? Also, what happens if the result is None, what do you return instead? Commented Oct 16, 2013 at 23:20
  • 1
    Is there other code in the function after the if? Commented Oct 16, 2013 at 23:20
  • Yes sorry it wasn't explicit, there is other code. This code is useful for example when you want to find solutions in a recursive function call. I'll write the example better Commented Oct 16, 2013 at 23:21
  • This looks like a question for codereview.stackexchange.com Commented Oct 16, 2013 at 23:21
  • 1
    @enrico.bacis: could you provide a complete example? I might have a reasonable idea but I'd like something specific to try it on. Commented Oct 17, 2013 at 0:11

6 Answers 6

Reset to default
13

Without knowing what else you might want to return there are a few options.

  1. You could just return the result of the function, None or not:

    return slow_function()

    In this, you rely on the caller knowing what to do with the None value, and really just shift where your logic will be.

  2. If you have a default value to return instead of None, you can do this:

    return slow_function() or default

    In this above, if slow_function is None (which is "falsy") it will return the latter value, otherwise, if slow_function returns a "truthy" value it will return that. Beware, if slow_function can return other "falsy" values, like False, [] or 0, those will be ignored.

  3. Alternatively, sometimes what you have is perfectly valid code. You want to compare against a value, and if it is value, return it. The code you have is obvious in what it does, and sometimes that is more important than the "cleverness" of the code.

As per the comments, if your code must continue running if the value is None then the most obvious way to do it is store it as a temporary value. But, thats not a bad thing, as it reads cleanly.

  • Compute a value and store it as the result
  • If there is a valid result, return it.
  • Otherwise, keep doing things to get a better result.

Better is usually very subjective, and I can't see any obvious ways to improve this from a computation perspective, and as written it is very human readable which is a clear advantage. Other solutions may be shorter or cleverer, but human readability is often an over looked advantage for code.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

As he said in the comment, he wants to return only if the function returns something different than None, otherwise the execution of the function must continue.
not the only way.. see my answer. i didn't downvote, but i don't get the upvotes. the first 2 the OP said not to do, and the 3rd just says it's impossible.
@Claudiu I've changed it from only to most obvious. The first two were written before the OP added extra details and depending on how long [...] is may be just as valid.
I'm sorry not to mark as accepted your answer but the first 2 alternatives are not feasible since they stop the evaluation and the third one (even if we all agree it's the way we should all write our code) is just what I was looking for an alternative. I thank you for your suggestions but I think @Claudiu made a better alternative, not the one with the exception (which I still upvoted for how he managed to force the language under his will) but the one with the yield, I think that one really add some value to the question (I also included a version of it in the question).
10

Your latest comment maybe makes it clearer what you want to do:

Imagine that you pass f a list and it select an item, then calls itself passing the list without the item and so on until you have no more items. The you check if the solution is feasible, if it is feasible you'll return the solution and this needs to go all the way through the call stack, otherwise you return None. In this way you'll explore all the problems in a topological order but you can also skip checks when you know that the previous chosen items won't be able to create a feasible solution.

Maybe you can try using yield instead of return. That is, your recursive function won't generate one solution, but will yield all possible solutions. Without a specific example I can't be sure what you're doing exactly, but say before it was like this:

def solve(args, result_so_far):
    if not slow_check_is_feasible(result_so_far):
        #dead-end
        return None

    if not args:
        #valid and done
        return result_so_far

    for i, item in enumerate(args):
        #pass list without args - slow
        new_args = args[:]
        del new_args[i]
        result = solve(new_args, accumulate_result(result_so_far, item)
        if result is not None:
            #found it, we are done
            return result
        #otherwise keep going

Now it looks like this:

def solve_all(args, result_so_far):
    if not slow_check_is_feasible(result_so_far):
        #dead-end
        return

    if not args:
        #yield since result was good
        yield result_so_far
        return

    for i, item in enumerate(args):
        #pass list without args - slow
        new_args = args[:]
        del new_args[i]
        for result in solve(new_args, accumulate_result(result_so_far, item):
            yield result

The benefits are:

  • You generate all answers instead of just the first one, but if you still only want one answer then you can just get the first result.
  • Before you used return values both for false checks and for answers. Now you're just only yielding when you have an answer.
Improve this answer

1 Comment

This is a really clever use of yield and something I can use safely in production. A really good answer, I also added a version of your code in the question. thank you!
7

Essentially you want to evaluate an expression and then use it twice without binding it to a local variable. The only way to do that, since we don't have anonymous variables, is to pass it into a function. Fortunately, the control flow for whether the current function returns isn't controlled by the functions it calls... however, exceptions do propagate up the call stack.

I wouldn't say this is better, but you could abuse exceptions to get what you want. This should never really be used and it's more an exercise in curiosity. The result would end up looking like this (note the use of the decorator):

def slow_function(x):
    if x % 5 == 0:
        return x * 200

@if_returner
def foobme(l):
    for i in l:
        print "Checking %s..." % (i,)
        return_if(slow_function(i))

print foobme([2, 3, 4, 5, 6])

Output is:

Checking 2...
Checking 3...
Checking 4...
Checking 5...
1000

The trick is to piggy-back on exception handling, since those propagate across function calls. If you like it, here's the implementation:

class ReturnExc(Exception):
    def __init__(self, val):
        self.val = val

def return_if(val):
    if val is not None:
        raise ReturnExc(val)

def if_returner(f):
    def wrapped(*args, **kwargs):
        try:
            return f(*args, **kwargs)
        except ReturnExc, e:
            return e.val
    return wrapped
Improve this answer

9 Comments

The use of decorators, exceptions (for non-exceptional code) and multiple function definitions is better than a single temporary variable how?
@LegoStormtroopr: I didn't say it was better. My answer even starts with "I wouldn't say this is better." But it was fun to come up with a way to actually accomplish what the OP wants. It is an interesting exercise to bend the language to your will sometimes. Besides this pattern isn't that crazy, it was inspired by Twisted's defer.returnValue(), and the multiple functions would obviously be reusable.
It pains me to downvote this, but while quite clever it is awful from a stylistic point of view. :)
Upvoted for novelty, as long as we have the understanding that I would slap you silly for deploying this in production. :-)
I have upvoted your answer because even if, as @KirkStrauser said, I'd never implement this in production, it's a really clever use of exception!
|
2

For the problem where slow_function is operating over a loop, a generator expression would seem the way to go. In Python 3 everything here is lazy, so you get your filter for free:

f = filter(slow_function(x) for x in range(...))

In Python 2 you just need itertools:

from itertools import ifilter

f = ifilter(slow_function(x) for x in xrange(...))

Each iteration will only take place when you ask for it. If you need to continue the operation if the function returns false, then you need the Falseness as a sentinel, so your solution is fine:

def f():
  for x in xrange(...):
    sentinel = slow_function(x)
    if sentinel:
      return sentinel
    # continue processing

or you can save yourself a variable by using a generator here, too:

from itertools import imap

def f():
  for x in imap(slow_function, xrange(...)):
    if x:
      return x
    # continue processing
Improve this answer

Comments

2

Not really a recommendation, but you could abuse a list comprehension and do something along these lines:

# Note: Doesn't work in python 3.
def func():
    if [value for value in (slow_function(),) if value is not None]:
        return value
    # continue processing...
Improve this answer

2 Comments

Well, this is closer, but wouldn’t the syntax have to be for value in [value for value in (slow_function(),) if value is not None]:? I get NameError: name 'value' is not defined for the reutrn value line otherwise. Also, if return value line is possible, that means that the solution trampled over any variable named value in the scope…
@binki: The syntax/logic of my original answer is valid for Python 2.x which "leaks" the list comprehension variable...and yes, it would change the value, if any, already associated with the variable. Fixing it for Python 3 would make it no better than the OP's original code.
-3

What you have written looks fine, but if you wanted to avoid multiple return statements you could do something like this:

def f():
    result = slow_function()
    if result is None:
        [...]
        result = [...]
    return result
Improve this answer

2 Comments

This is a very anti pattern and depending on how long the [...] section is the return and its logic might now be quite far apart.
I had a colleague who did this all the time so that there was never a question of where the function was exiting. If your function is small readability is not an issue, but I can see how it can easily get out of hand. See stackoverflow.com/questions/36707/…

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.