ChatGPT解决这个技术问题 Extra ChatGPT

Pythonic way to avoid "if x: return x" statements

I have a method that calls 4 other methods in sequence to check for specific conditions, and returns immediately (not checking the following ones) whenever one returns something Truthy.

def check_all_conditions():
    x = check_size()
    if x:
        return x

    x = check_color()
    if x:
        return x

    x = check_tone()
    if x:
        return x

    x = check_flavor()
    if x:
        return x
    return None

This seems like a lot of baggage code. Instead of each 2-line if statement, I'd rather do something like:

x and return x

But that is invalid Python. Am I missing a simple, elegant solution here? Incidentally, in this situation, those four check methods may be expensive, so I do not want to call them multiple times.

What are these x's? Are they just True/False, or are they data structures containing some information, with None or similar being used as a special case to indicate the absence of any data? If it's the latter, you should almost certainly be using exceptions instead.
@gerrit The code as presented above is hypothetical/pseudo code which is off-topic on Code Review. If the author of the post wish to get their real, actual working code reviewed, then yes they are welcome to post on Code Review.
Why do you think x and return x is better than if x: return x? The latter is far more readable and thus maintainable. You shouldn't worry too much about the number of characters or lines; readability counts. They're the exact same number of non-whitespace characters anyway, and if you really must, if x: return x will work fine on just one line.
Please clarify whether you care about the actual values or you really just need to return a boolean. This makes a difference what options are available and also which ones more clearly communicate the intent. The naming suggests you only need a boolean. It also makes a difference whether avoiding multiple calls to these functions is important. It could also matter if the functions take any or different sets of parameters. Without these clarifications, I think this question falls into one of Unclear, Too Broad, or Opinion Based.
@jpmc26 OP explicitly speaks of truthy return values, and then his code returns x (as opposed to bool(x)) so as it stands I think it is safe to assume that OP's functions can return anything, and he wants the first anything that's truthy.

C
CompuChip

Alternatively to Martijn's fine answer, you could chain or. This will return the first truthy value, or None if there's no truthy value:

def check_all_conditions():
    return check_size() or check_color() or check_tone() or check_flavor() or None

Demo:

>>> x = [] or 0 or {} or -1 or None
>>> x
-1
>>> x = [] or 0 or {} or '' or None
>>> x is None
True

Sure, but this will get tedious to read fast if there are more than a few options. Plus my approach lets you use a variable number of conditions.
@MartijnPieters you can use \ to put each check on its own line.
@MartijnPieters I never implied my answer is better than yours, I like your answer, too :)
@Caridorc: I strongly dislike using \ to extend the logical line. Use parentheses where possible instead; so return (....) with newlines inserted as needed. Still, that'll be one long logical line.
I think this is the better solution. The argument "it will get tedious [..] if there are more than a few options" is moot, because a single function should not be making an exorbitant number of checks anyways. If that is required, the checks should be split into multiple functions.
M
Martijn Pieters

You could use a loop:

conditions = (check_size, check_color, check_tone, check_flavor)
for condition in conditions:
    result = condition()
    if result:
        return result

This has the added advantage that you can now make the number of conditions variable.

You could use map() + filter() (the Python 3 versions, use the future_builtins versions in Python 2) to get the first such matching value:

try:
    # Python 2
    from future_builtins import map, filter
except ImportError:
    # Python 3
    pass

conditions = (check_size, check_color, check_tone, check_flavor)
return next(filter(None, map(lambda f: f(), conditions)), None)

but if this is more readable is debatable.

Another option is to use a generator expression:

conditions = (check_size, check_color, check_tone, check_flavor)
checks = (condition() for condition in conditions)
return next((check for check in checks if check), None)

if the conditions are really only conditions, i.e. booleans then in your first proposal you could also use the builtin any instead of the loop. return any(condition() for condition in conditions)
@Leonhard: any has almost the same implementation inside. But it looks much better, please post it as an answer )
Readability trumps almost all other considerations. You say, map/filter is 'debatable', I put my vote in for undebatably ugly. Thanks, certainly, but if anyone on my team put a map/filter in for this code I'd transfer them to another team or assign them to bedpan duty.
Is this unreadable block of code really "pythonian"? And especially the idea of zipping conditions and arguments? This is IMHO much worse than the original code, which takes about 10 seconds to parse by my brainparser.
"Prefer Python", they said. "Perl is unreadable", they said. And then this happened: return next((check for check in checks if check), None).
J
Jack Aidley

Don't change it

There are other ways of doing this as the various other answers show. None are as clear as your original code.


I'd argue against that, but your suggestion is a legitimate one to be voiced. Personally, I find my eyes strained trying to read the OP while, for example, timgeb's solution clicks instantly.
It's really a matter of opinion. Me personally, I would remove the newlines after :, because I consider if x: return x to be pretty fine, and it makes the function look more compact. But that may be just me.
It's not just you. Using or as timgeb did is a proper and well-understood idiom. Many languages have this; perhaps when it is called orelse it is even more clear, but even plain old or (or || in other languages) is meant to be understood as the alternative to try if the first one "doesn't work."
@RayToal: Importing idioms from other languages is a great way to obfuscate code.
Sometimes yes, for sure! Also it can be a way to open one's mind and lead one to discover new and better patterns and paradigms that no one may have tried before. Style evolves by people borrowing and sharing and trying new things. Works both ways. Anyway, I've never heard the use of or labeled un-Pythonic or in any way obfuscated, but that's a matter of opinion anyway---as it should be.
W
Wayne Werner

In effectively the same answer as timgeb, but you could use parenthesis for nicer formatting:

def check_all_the_things():
    return (
        one()
        or two()
        or five()
        or three()
        or None
    )

everybody please help raising this answer up to 1st place. do your part !
P
Phil Frost

According to Curly's law, you can make this code more readable by splitting two concerns:

What things do I check?

Has one thing returned true?

into two functions:

def all_conditions():
    yield check_size()
    yield check_color()
    yield check_tone()
    yield check_flavor()

def check_all_conditions():
    for condition in all_conditions():
        if condition:
            return condition
    return None

This avoids:

complicated logical structures

really long lines

repetition

...while preserving a linear, easy to read flow.

You can probably also come up with even better function names, according to your particular circumstance, which make it even more readable.


I like this one, although True/False should be changed to condition/None to match the question.
This is my favourite! It copes with different checks and arguments too. Quite possibly overdesigned for this particular example but a really useful tool for future problems!
Note that return None is not necessary, because functions return None by default. However, there's nothing wrong with returning None explicitly, and I like that you chose to do so.
I think this approach would be better implemented with a local function definition.
@timgeb "Explicit is better than implicit," Zen of Python.
佚名

This is a variant of Martijns first example. It also uses the "collection of callables"-style in order to allow short-circuiting.

Instead of a loop you can use the builtin any.

conditions = (check_size, check_color, check_tone, check_flavor)
return any(condition() for condition in conditions) 

Note that any returns a boolean, so if you need the exact return value of the check, this solution will not work. any will not distinguish between 14, 'red', 'sharp', 'spicy' as return values, they will all be returned as True.


You could do next(itertools.ifilter(None, (c() for c in conditions))) to get the actual value without casting it to a boolean.
Does any actually short-circuit?
@zwol Yes try it with some sample functions or see docs.python.org/3/library/functions.html
This is less readable than chaining the 4 functions with 'or' and only pays off if the number of conditions is large or dynamic.
@rjh It's perfectly readable; it's just a list literal and a comprehension. I'd prefer it because my eyes glazeth over after about the third x = bar(); if x: return x;
z
zwol

Have you considered just writing if x: return x all on one line?

def check_all_conditions():
    x = check_size()
    if x: return x

    x = check_color()
    if x: return x

    x = check_tone()
    if x: return x

    x = check_flavor()
    if x: return x

    return None

This isn't any less repetitive than what you had, but IMNSHO it reads quite a bit smoother.


A
Arnaud Le Blanc

I'm quite surprised nobody mentioned the built-in any which is made for this purpose:

def check_all_conditions():
    return any([
        check_size(),
        check_color(),
        check_tone(),
        check_flavor()
    ])

Note that although this implementation is probably the clearest, it evaluates all the checks even if the first one is True.

If you really need to stop at the first failed check, consider using reduce which is made to convert a list to a simple value:

def check_all_conditions():
    checks = [check_size, check_color, check_tone, check_flavor]
    return reduce(lambda a, f: a or f(), checks, False)

reduce(function, iterable[, initializer]) : Apply function of two arguments cumulatively to the items of iterable, from left to right, so as to reduce the iterable to a single value. The left argument, x, is the accumulated value and the right argument, y, is the update value from the iterable. If the optional initializer is present, it is placed before the items of the iterable in the calculation

In your case:

lambda a, f: a or f() is the function that checks that either the accumulator a or the current check f() is True. Note that if a is True, f() won't be evaluated.

checks contains check functions (the f item from the lambda)

False is the initial value, otherwise no check would happen and the result would always be True

any and reduce are basic tools for functional programming. I strongly encourage you to train these out as well as map which is awesome too!


any only works if the checks actually return a boolean value, literally True or False, but the question doesn't specify that. You'd need to use reduce to return the actual value returned by the check. Also, it's easy enough to avoid evaluating all the checks with any by using a generator, e.g. any(c() for c in (check_size, check_color, check_tone, check_flavor)). As in Leonhard's answer
I like your explanation and usage of reduce. Like @DavidZ I believe your solution with any should use a generator and it needs to be pointed out that it is limited to returning True or False.
@DavidZ actually any works with truthy values: any([1, "abc", False]) == True and any(["", 0]) == False
@blint sorry, I wasn't clear. The goal of the question is to return the result of the check (and not merely to indicate whether the check succeeded or failed). I was pointing out that any only works for that purpose if actual boolean values are returned from the check functions.
P
Phinet

If you want the same code structure, you could use ternary statements!

def check_all_conditions():
    x = check_size()
    x = x if x else check_color()
    x = x if x else check_tone()
    x = x if x else check_flavor()

    return x if x else None

I think this looks nice and clear if you look at it.

Demo:

https://i.stack.imgur.com/gYcy8.png


Whats with the little ASCII fish above your terminal?
@LegoStormtroopr I use the fish shell, so I decorate it with an ascii fish tank to make me happy. :)
Thanks for the fine fish (and the colors by the way, what editor is that?)
You can get fish at fishshell.com, and the config file for the ascii here pastebin.com/yYVYvVeK, also the editor is sublime text.
x if x else <something> can just be reduced to x or <something>
j
juandesant

For me, the best answer is that from @phil-frost, followed by @wayne-werner's.

What I find interesting is that no one has said anything about the fact that a function will be returning many different data types, which will make then mandatory to do checks on the type of x itself to do any further work.

So I would mix @PhilFrost's response with the idea of keeping a single type:

def all_conditions(x):
    yield check_size(x)
    yield check_color(x)
    yield check_tone(x)
    yield check_flavor(x)

def assessed_x(x,func=all_conditions):
    for condition in func(x):
        if condition:
            return x
    return None

Notice that x is passed as an argument, but also all_conditions is used as a passed generator of checking functions where all of them get an x to be checked, and return True or False. By using func with all_conditions as default value, you can use assessed_x(x), or you can pass a further personalised generator via func.

That way, you get x as soon as one check passes, but it will always be the same type.


R
RoadieRich

Ideally, I would re-write the check_ functions to return True or False rather than a value. Your checks then become

if check_size(x):
    return x
#etc

Assuming your x is not immutable, your function can still modify it (although they can't reassign it) - but a function called check shouldn't really be modifying it anyway.


R
Reza Dodge

I like @timgeb's. In the meantime I would like to add that expressing None in the return statement is not needed as the collection of or separated statements are evaluated and the first none-zero, none-empty, none-None is returned and if there isn't any then None is returned whether there is a None or not!

So my check_all_conditions() function looks like this:

def check_all_conditions():
    return check_size() or check_color() or check_tone() or check_flavor()

Using timeit with number=10**7 I looked at the running time of a number of the suggestions. For the sake of comparison I just used the random.random() function to return a string or None based on random numbers. Here is the whole code:

import random
import timeit

def check_size():
    if random.random() < 0.25: return "BIG"

def check_color():
    if random.random() < 0.25: return "RED"

def check_tone():
    if random.random() < 0.25: return "SOFT"

def check_flavor():
    if random.random() < 0.25: return "SWEET"

def check_all_conditions_Bernard():
    x = check_size()
    if x:
        return x

    x = check_color()
    if x:
        return x

    x = check_tone()
    if x:
        return x

    x = check_flavor()
    if x:
        return x
    return None

def check_all_Martijn_Pieters():
    conditions = (check_size, check_color, check_tone, check_flavor)
    for condition in conditions:
        result = condition()
        if result:
            return result

def check_all_conditions_timgeb():
    return check_size() or check_color() or check_tone() or check_flavor() or None

def check_all_conditions_Reza():
    return check_size() or check_color() or check_tone() or check_flavor()

def check_all_conditions_Phinet():
    x = check_size()
    x = x if x else check_color()
    x = x if x else check_tone()
    x = x if x else check_flavor()

    return x if x else None

def all_conditions():
    yield check_size()
    yield check_color()
    yield check_tone()
    yield check_flavor()

def check_all_conditions_Phil_Frost():
    for condition in all_conditions():
        if condition:
            return condition

def main():
    num = 10000000
    random.seed(20)
    print("Bernard:", timeit.timeit('check_all_conditions_Bernard()', 'from __main__ import check_all_conditions_Bernard', number=num))
    random.seed(20)
    print("Martijn Pieters:", timeit.timeit('check_all_Martijn_Pieters()', 'from __main__ import check_all_Martijn_Pieters', number=num))
    random.seed(20)
    print("timgeb:", timeit.timeit('check_all_conditions_timgeb()', 'from __main__ import check_all_conditions_timgeb', number=num))
    random.seed(20)
    print("Reza:", timeit.timeit('check_all_conditions_Reza()', 'from __main__ import check_all_conditions_Reza', number=num))
    random.seed(20)
    print("Phinet:", timeit.timeit('check_all_conditions_Phinet()', 'from __main__ import check_all_conditions_Phinet', number=num))
    random.seed(20)
    print("Phil Frost:", timeit.timeit('check_all_conditions_Phil_Frost()', 'from __main__ import check_all_conditions_Phil_Frost', number=num))

if __name__ == '__main__':
    main()

And here are the results:

Bernard: 7.398444877040768
Martijn Pieters: 8.506569201346597
timgeb: 7.244275416364456
Reza: 6.982133448743038
Phinet: 7.925932800076634
Phil Frost: 11.924794811353031

m
mathreadler

A slight variation on Martijns first example above, that avoids the if inside the loop:

Status = None
for c in [check_size, check_color, check_tone, check_flavor]:
  Status = Status or c();
return Status

Does it? You still do a comparison. In your version you will also check all conditions regardless and not return at the first instance of a truthy value, Dependng on how costly those functions are, that may not be desirable.
@Reti43: Status or c() will skip/short-circui evaluate calls to c() if Status is truthy, so the code in this answer does not appear to call any more functions than the OP's code. stackoverflow.com/questions/2580136/…
@NeilSlater True. The only downside I see is that the best case is now in O(n) because the listiterator has to yield n times, when it was O(1) before if the first function returns something truthy in O(1).
Yes good points. I just have to hope that the c() takes a bit more time to evaluate than looping an almost-empty loop. Checking flavour could take a whole evening at least if it's a good one.
R
Rick supports Monica

This way is a little bit outside of the box, but I think the end result is simple, readable, and looks nice.

The basic idea is to raise an exception when one of the functions evaluates as truthy, and return the result. Here's how it might look:

def check_conditions():
    try:
        assertFalsey(
            check_size,
            check_color,
            check_tone,
            check_flavor)
    except TruthyException as e:
        return e.trigger
    else:
        return None

You'll need a assertFalsey function that raises an exception when one of the called function arguments evaluates as truthy:

def assertFalsey(*funcs):
    for f in funcs:
        o = f()
        if o:
            raise TruthyException(o)

The above could be modified so as to also provide arguments for the functions to be evaluated.

And of course you'll need the TruthyException itself. This exception provides the object that triggered the exception:

class TruthyException(Exception):
    def __init__(self, obj, *args):
        super().__init__(*args)
        self.trigger = obj

You can turn the original function into something more general, of course:

def get_truthy_condition(*conditions):
    try:
        assertFalsey(*conditions)
    except TruthyException as e:
        return e.trigger
    else:
        return None

result = get_truthy_condition(check_size, check_color, check_tone, check_flavor)

This might be a bit slower because you are using both an if statement and handling an exception. However, the exception is only handled a maximum of one time, so the hit to performance should be minor unless you expect to run the check and get a True value many many thousands of times.


// , Cute! Is it considered "Pythonic" to use exception handling for this sort of thing?
@NathanBasanese Sure- exceptions are used for control flow all the time. StopIteration is a pretty good example: an exception is raised every single time you exhaust an iterable. The thing you want to avoid is successively raising exceptions over and over again, which would get expensive. But doing it one time is not.
// , Ah, I take it you're referring to something like programmers.stackexchange.com/questions/112463/…. I have voted for that question and for this answer. Python 3 docs for this are here: docs.python.org/3/library/stdtypes.html#iterator-types, I think.
You want to define a general-purpose function and an exception, just to do a few checks in some other function somewhere? I think that's a bit much.
@BacklightShining I agree. I'd never actually do this myself. The OP asked for ways to avoid the repeated code, but I think what he started with is perfectly fine.
D
Dmitry Rubanovich

The pythonic way is either using reduce (as someone already mentioned) or itertools (as shown below), but it seems to me that simply using short circuiting of the or operator produces clearer code

from itertools import imap, dropwhile

def check_all_conditions():
    conditions = (check_size,\
        check_color,\
        check_tone,\
        check_flavor)
    results_gen = dropwhile(lambda x:not x, imap(lambda check:check(), conditions))
    try:
        return results_gen.next()
    except StopIteration:
        return None

z
zwol

If you can require Python 3.8, you can use the new feature of "assignment expressions" to make the if-else chain somewhat less repetitive:

def check_all_conditions():
    if (x := check_size()): return x
    if (x := check_color()): return x
    if (x := check_tone()): return x
    if (x := check_flavor()): return x
    
    return None

It's not valid Python, no. Python doesn't let you use the assignment operator like that. However, a new special assignment expression was added very recently, so you can now write if ( x := check_size() ) : for the same effect.
U
U12-Forward

Or use max:

def check_all_conditions():
    return max(check_size(), check_color(), check_tone(), check_flavor()) or None

C
Carel

I have seen some interesting implementations of switch/case statements with dicts in the past that led me to this answer. Using the example you've provided you would get the following. (It's madness using_complete_sentences_for_function_names, so check_all_conditions is renamed to status. See (1))

def status(k = 'a', s = {'a':'b','b':'c','c':'d','d':None}) :
  select = lambda next, test : test if test else next
  d = {'a': lambda : select(s['a'], check_size()  ),
       'b': lambda : select(s['b'], check_color() ),
       'c': lambda : select(s['c'], check_tone()  ),
       'd': lambda : select(s['d'], check_flavor())}
  while k in d : k = d[k]()
  return k

The select function eliminates the need to call each check_FUNCTION twice i.e. you avoid check_FUNCTION() if check_FUNCTION() else next by adding another function layer. This is useful for long running functions. The lambdas in the dict delay execution of it's values until the while loop.

As a bonus you may modify the execution order and even skip some of the tests by altering k and s e.g. k='c',s={'c':'b','b':None} reduces the number of tests and reverses the original processing order.

The timeit fellows might haggle over the cost of adding an extra layer or two to the stack and the cost for the dict look up but you seem more concerned with the prettiness of the code.

Alternatively a simpler implementation might be the following :

def status(k=check_size) :
  select = lambda next, test : test if test else next
  d = {check_size  : lambda : select(check_color,  check_size()  ),
       check_color : lambda : select(check_tone,   check_color() ),
       check_tone  : lambda : select(check_flavor, check_tone()  ),
       check_flavor: lambda : select(None,         check_flavor())}
  while k in d : k = d[k]()
  return k

I mean this not in terms of pep8 but in terms of using one concise descriptive word in place of a sentence. Granted the OP may be following some coding convention, working one some existing code base or not care for terse terms in their codebase.


Sometimes people go really crazy with their naming when one word will do. Using the OP's code as an example it is unlikely that he would have functions called check_no/some/even/prime/every_third/fancy_conditions but just this one function so why not call it status or if one insists check_status. Using _all_ is superfluous, he's not ensuring the universes integrity. Naming should surely use a consistent set of keywords leveraging name spacing whenever possible. Lengthy sentences best serve as docstrings. One seldomly needs more then 8-10 characters to describe something succinctly.
I'm a fan of long function names, because I want higher-level functions to be self-documenting. But check_all_conditions is a bad name, because it's not checking all conditions if one is true. I'd use something like matches_any_condition.
That is an interesting tact to take. I try to minimize the number of letters I'll make typo's on later :) It seems I've dolloped a heap of opinion into my solution, when I was really trying to provide a helpful hint. Should this be edited out ?
This seems way too hacky, especially considering the other solutions on this question. What OP is trying to do isn't complicated at all; the solution should be simple enough to understand half-asleep. And I have no idea what's going on here.
I was aiming for flexibility. Modified answer to include a less 'hacky' variant