Question

I've been refactoring some rather crufty code and came across the following rather odd construct:

#!/usr/bin/env python2.7
# ...
if (opts.foo or opts.bar or opts.baz) is None:
    # (actual option names changed to protect the guilty)
    sys.stderr.write("Some error messages that these are required arguments")

... and I was wondering if this would ever make any conceivable sense.

I changed it to something like:

#!/usr/bin/env python2.7 
if None in (opts.foo, opts.bar, opts.baz):
    # ...

I did fire up an interpreter and actually try the first construct ... it only seems to work if the values are all false and the last of these false values is None. (In other words CPython's implementation seems to return the first true or last false value from a chain of or expressions).

I still suspect that the proper code should use either the any() or all() built-ins which were added 2.5 (the code in question already requires 2.7). I'm not yet sure which are the preferred/intended semantics as I'm just starting on this project.

So is there any case where this original code would make sense?

Was it helpful?

Solution

The short-circuiting behavior causes foo or bar or baz to return the first of the three values that is boolean-true, or the last value if all are boolean-false. So it basically means "if all are false and the last one is None".

Your changed version is slightly different. if None in (opts.foo, opts.bar, opts.baz) will, for instance, enter the if block if opts.foo is None and the other two are 1, whereas the original version will not (because None or 1 or 1 will evaluate to 1, which is not None). Your version will enter the if when any of the three is None, regardless of what the other two are, whereas the original version will enter the if only if the last is None and the other two are any boolean-false values.

Which of the two versions you want depends on how the rest of the code is structured and what values the options might take (in particular, whether they might have boolean-false values other than None, such as False or 0 or an empty string). Intuitively your version does seem more reasonable, but if the code has peculiar tricks like this in it, you never know what corner cases might emerge.

OTHER TIPS

It's behaving that way because or is an short-circuit operator, details are in docs. Thus your first if statement is equal to:

if opts.baz is None

We could guess what author of that code expected. I think that, as you mentioned, he thought of using not all([opts.foo, opts.bar, opts.baz]).

I would prefer

 if any(i is None for i in (opts.foo, opts.bar, opts.baz))

as it exactly expresses the intended goal.

OTOH,

not all([opts.foo, opts.bar, opts.baz])

does check for falseness, not for None.

The original code doesn't seem to make sense; it seems to have been written by someone unaware what they are doing.

let's try both two of your code:

In [20]: foo = True

In [22]: bar = None

In [23]: baz = None

In [24]: foo or bar or baz
Out[24]: True

In [25]: (foo or bar or baz) is None
Out[25]: False

In [28]: ((foo or bar or baz) is None) == (None in (foo, bar, baz))
Out[28]: False

You can see your rewrite is not same as the original code.

Your first condition only return True when all of your variables is None

In [19]: (None or None or None) is None
Out[19]: True

so you can rewrite your first condition to:

if foo == bar == bar == None:
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top