if (foo or bar or baz) is None:
Asked Answered
D

4

8

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?

Darmit answered 5/4, 2013 at 7:35 Comment(1)
That's terrible. be careful you don't introduce a new bug by fixing this :)Observatory
D
5

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.

Dwan answered 5/4, 2013 at 7:47 Comment(0)
U
4

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]).

Unwary answered 5/4, 2013 at 7:38 Comment(2)
To answer his question more specifically: it does seem like the code is in error and I can't think of a case where it would make sense.Ciliolate
It would make sense if there were no "is" operator there. But this is technically testing if all values evaluate to false and the last is, specifically, a reference to the None singleton.Darmit
M
0

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.

Mousterian answered 5/4, 2013 at 7:54 Comment(0)
K
0

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:
Kenakenaf answered 5/4, 2013 at 8:0 Comment(1)
I acknowledged that the semantics are different; the point here is to speculate on whether there are any plausible cases for the original semantics to be correct. I think the previous code would have failed in various corner cases which, happily, never seem to have been noted in practice. (It's part of an internal command line utility which has only been used by a very few staff members).Darmit

© 2022 - 2024 — McMap. All rights reserved.