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.
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.
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.
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
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)
any
instead of the loop. return any(condition() for condition in conditions)
any
has almost the same implementation inside. But it looks much better, please post it as an answer )
conditions
and arguments
? This is IMHO much worse than the original code, which takes about 10 seconds to parse by my brainparser.
return next((check for check in checks if check), None)
.
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.
:
, 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.
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."
or
labeled un-Pythonic or in any way obfuscated, but that's a matter of opinion anyway---as it should be.
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
)
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.
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.
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
.
next(itertools.ifilter(None, (c() for c in conditions)))
to get the actual value without casting it to a boolean.
any
actually short-circuit?
x = bar(); if x: return x;
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.
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
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
.
any
works with truthy values: any([1, "abc", False]) == True
and any(["", 0]) == False
any
only works for that purpose if actual boolean values are returned from the check functions.
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
x if x else <something>
can just be reduced to x or <something>
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.
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.
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
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
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/…
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.
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.
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
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
if ( x := check_size() ) :
for the same effect.
Or use max
:
def check_all_conditions():
return max(check_size(), check_color(), check_tone(), check_flavor()) or None
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.
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.
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
.
Success story sharing
\
to put each check on its own line.\
to extend the logical line. Use parentheses where possible instead; soreturn (....)
with newlines inserted as needed. Still, that'll be one long logical line.