170
votes

I have a function that can return one of three things:

  • success (True)
  • failure (False)
  • error reading/parsing stream (None)

My question is, if I'm not supposed to test against True or False, how should I see what the result is. Below is how I'm currently doing it:

result = simulate(open("myfile"))
if result == None:
    print "error parsing stream"
elif result == True: # shouldn't do this
    print "result pass"
else:
    print "result fail"

is it really as simple as removing the == True part or should I add a tri-bool data-type. I do not want the simulate function to throw an exception as all I want the outer program to do with an error is log it and continue.

6
You are asking the wrong question; you should be asking for help defining your result ... what is the difference that you perceive between "failure" and "error parsing stream", what do they mean, what are the consequences, what action is the caller likely to want to take in each case (pass, fail, parse error)? - John Machin
I'm simulating an electrical power system, if people lose power to their houses it is a failure. If I can't read the simulation file then that is an error of a completely different kind. - James Brooks
Inside the simulate function I catch all exceptions; I don't want anything that happens inside the simulator to stop the rest of the program running (and processing the next element). But the answers are making me change my mind. - James Brooks
@James Brooks: Right. That's what try/except processing is all about. If your simulate has things it can catch and retry, that's good. But if it "fails", it should not return None. It should just raise an exception to the script that called it. Either way, simulate is done. Returning None isn't as helpful as raising a proper exception -- or allowing an exception to propagate through simulate into the calling script for handling. - S.Lott
@James, use except Exception: instead. This catches all "real" errors, along with Warning and StopIteration. It allows KeyboardInterrupt and SystemExit through though. If you really want to catch those, it's probably best to use another, outer try/except or some other structure that clearly documents your intent, since those are not "errors". (But I did say "almost never"... perhaps in your case you really do want to grab everything, and even prevent Ctrl-C or sys.exit() from exiting, etc.) - Peter Hansen

6 Answers

129
votes

Don't fear the Exception! Having your program just log and continue is as easy as:

try:
    result = simulate(open("myfile"))
except SimulationException as sim_exc:
    print "error parsing stream", sim_exc
else:
    if result:
        print "result pass"
    else:
        print "result fail"

# execution continues from here, regardless of exception or not

And now you can have a much richer type of notification from the simulate method as to what exactly went wrong, in case you find error/no-error not to be informative enough.

197
votes
if result is None:
    print "error parsing stream"
elif result:
    print "result pass"
else:
    print "result fail"

keep it simple and explicit. You can of course pre-define a dictionary.

messages = {None: 'error', True: 'pass', False: 'fail'}
print messages[result]

If you plan on modifying your simulate function to include more return codes, maintaining this code might become a bit of an issue.

The simulate might also raise an exception on the parsing error, in which case you'd either would catch it here or let it propagate a level up and the printing bit would be reduced to a one-line if-else statement.

14
votes

Never, never, never say

if something == True:

Never. It's crazy, since you're redundantly repeating what is redundantly specified as the redundant condition rule for an if-statement.

Worse, still, never, never, never say

if something == False:

You have not. Feel free to use it.

Finally, doing a == None is inefficient. Do a is None. None is a special singleton object, there can only be one. Just check to see if you have that object.

6
votes

There are many good answers. I would like to add one more point. A bug can get into your code if you are working with numerical values, and your answer is happened to be 0.

a = 0 
b = 10 
c = None

### Common approach that can cause a problem

if not a:
    print(f"Answer is not found. Answer is {str(a)}.") 
else:
    print(f"Answer is: {str(a)}.")

if not b:
    print(f"Answer is not found. Answer is {str(b)}.") 
else:
    print(f"Answer is: {str(b)}")

if not c:
    print(f"Answer is not found. Answer is {str(c)}.") 
else:
    print(f"Answer is: {str(c)}.")
Answer is not found. Answer is 0.   
Answer is: 10.   
Answer is not found. Answer is None.
### Safer approach 
if a is None:
    print(f"Answer is not found. Answer is {str(a)}.") 
else:
    print(f"Answer is: {str(a)}.")

if b is None:
    print(f"Answer is not found. Answer is {str(b)}.") 
else:
    print(f"Answer is: {str(b)}.")

if c is None:
    print(f"Answer is not found. Answer is {str(c)}.") 
else:
    print(f"Answer is: {str(c)}.")

Answer is: 0.
Answer is: 10.
Answer is not found. Answer is None.
3
votes

I would like to stress that, even if there are situations where if expr : isn't sufficient because one wants to make sure expr is True and not just different from 0/None/whatever, is is to be prefered from == for the same reason S.Lott mentionned for avoiding == None.

It is indeed slightly more efficient and, cherry on the cake, more human readable.

In [1]: %timeit (1 == 1) == True
38.1 ns ± 0.116 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)

In [2]: %timeit (1 == 1) is True
33.7 ns ± 0.141 ns per loop (mean ± std. dev. of 7 runs, 10000000 loops each)
1
votes

I believe that throwing an exception is a better idea for your situation. An alternative will be the simulation method to return a tuple. The first item will be the status and the second one the result:

result = simulate(open("myfile"))
if not result[0]:
  print "error parsing stream"
else:
  ret= result[1]