250

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.

Improve this question
14
  • 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)? Commented Jan 7, 2010 at 14:01
  • 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. Commented Jan 7, 2010 at 14:06
  • 2
    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. Commented Jan 7, 2010 at 15:06
  • 1
    @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. Commented Jan 7, 2010 at 15:34
  • 2
    @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.) Commented Jan 22, 2010 at 17:41

6 Answers 6

Reset to default
270 
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.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Image
The latter is kind of an explicit test against True or False, isn't it?
Image
of course, but knowing that these are only possible return values, I don't think it's a problem.
Image
and it seem to be a bit faster as well
Image
a = 'foo' if a: print 'its true' a is not actually TRUE, it's just not none
Image
and just not False
144

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.

Improve this answer

7 Comments

Image
Agreed. Much more pythonic than the evidently more popular solution above (which smells too much like C code).
Image
@Brandon Not agreed. This code is longer and, worse, less readable than the solution above (or the improved version below): more indentations, more different statements - guess why the latter is more popular, as you say ... ;-) Why trying to be 'Pythonic' if that leads to more awkward code ...?
Image
Now print the traceback instead of "error parsing stream" and you got my vote.
Image
Many people will come to this page looking for the answer to the title question. For most of us "Don't fear the Exception!" has nothing to do with our situation. We just need to test for True, False, and None. While your suggested alternative is valid for some cases, I think it's best to also include an answer to the question as it was asked.
Image
I disagree, I prefer to write if (result is None): rather than try: [. . .] except SimulationException as sim_exc: [. . .] else:
|
14

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.
Improve this answer

Comments

12

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.

Improve this answer

11 Comments

Image
Testing for equality with True is not redundant (although I agree it's not sensible). It could be calling an __eq__ or other special method, which could do practically anything.
Image
@Scott Griffiths: Good point. That's a truly and deeply horrifying scenario. If that's actually the case, the program violates our fundamental expectations in a way that makes it something that needs to be simply deleted and rewritten from scratch without such black magic.
Image
'Never, never, never' ...? There are cases though that if something == True yields a different result than if something, e.g. for non-boolean something. 2==True yields false whereas 2 evaluates to true; None==False is false but not None is true!
Image
-1 This answer misleading and completely incorrect, since what @Rolf Bartstra says is true. Although in this case, what you say can be applied.
Image
-1. Since any non-zero or non-empty or non-zero-length value for something returns True on bool(something). In that case, if you ONLY want to check if something has a value of True i.e. bool. Then you HAVE to do if something == True IMO.
|
4

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)
Improve this answer

1 Comment

Image
You can not run a benchmark once and say that one is more efficient than the other one (even though it might be). Run it many times (10.000) to see how it behaves in average.\
2

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]
Improve this answer

4 Comments

Image
returning tuple usually goes well with unpacking a tuple ;)
Image
your code, however, doesn't make much sense, if False is returned, it'll print 'error parsing stream'.
Image
The simulate method should return (False, "anything at all") or (True, ret) where ret is either False or True.
Image
well, you're re-defining the output values to suit your logic, it isn't clear w/o an explanation

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.