220

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.

James Brooks
  • 4,135
  • 4
  • 26
  • 25
  • 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 Jan 07 '10 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. – James Brooks Jan 07 '10 at 14:06
  • It's unclear (possibly because it isn't really a good idea) why you'd want to avoid using an exception to indicate an exceptional condition. Has something convinced you that exceptions are "heavy-weight", slow, or have some other problem for this sort of case? Paul's answer shows how one should do this with Python. – Peter Hansen Jan 07 '10 at 14:48
  • 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. – James Brooks Jan 07 '10 at 15:06
  • Exceptions are a good thing. They're how you do things like validate input, debug, fix your design, alert the user's that their problem is ill-constrained, etc., etc. – S.Lott Jan 07 '10 at 15:13
  • The problem is I'm going to be leaving this program running overnight. I don't want it to stop when there is an error; just log it and carry on. Then in the morning I can fix the error and just re-run those ones. – James Brooks Jan 07 '10 at 15:22
  • 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. – S.Lott Jan 07 '10 at 15:34
  • I see. If `simulate` were an external library then I would want it to throw an exception. It just fits better. In my case the calling code can then `except` everything, log it and move on. (as per Paul McGuire answer) – James Brooks Jan 07 '10 at 15:42
  • @James, except that a blanket `except:` statement is also a very bad idea. Among other problems, it will catch exceptions that are not descendants of `StandardError` in the exception class hierarchy, which you almost never want to do. – Peter Hansen Jan 22 '10 at 14:26
  • @Peter, why is it a bad idea? If anything at all causes the inner simulator to throw then I want to log that as an error. I really don't want my program to stop processing just because one of the things it's simulating threw an error I didn't expect. What should I do in this case? – James Brooks Jan 22 '10 at 14:48
  • 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.) – Peter Hansen Jan 22 '10 at 17:41
  • Thanks for the clarification. I think your right, I shouldn't catch `KeyboardInterrupt` (and maybe others) but I don't want to risk not catching other things (maybe things that don't properly inherit from Exception). Basically I realise it's bad and I'm going to do it anyway. – James Brooks Jan 22 '10 at 19:30
  • downvoted. Please correct the title to reflect the underlying question and the accepted answer. – danorton Feb 09 '13 at 00:37

6 Answers6

238
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.

SilentGhost
  • 307,395
  • 66
  • 306
  • 293
135

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.

PaulMcG
  • 62,419
  • 16
  • 94
  • 130
  • Agreed. Much more pythonic than the evidently more popular solution above (which smells too much like C code). – Brandon Jan 07 '10 at 13:53
  • 10
    @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 ...? – Rolf Bartstra Dec 06 '12 at 16:47
  • Now print the traceback instead of "error parsing stream" and you got my vote. – CivFan Oct 14 '15 at 19:34
  • Ok, you got my vote anyway, but I meant print something like `traceback.format_exc() `. [See this SO answer.](http://stackoverflow.com/a/3702847/2125392) – CivFan Oct 22 '15 at 19:32
  • Printing a traceback isn't *always* the way to go, especially if this is an area where the output is conveyed to the end user. Log tracebacks, sure, but they are undesirable gobbledygook to the user - they want a summary error message, preferably with advice on what they can do to fix the error, if anything. – PaulMcG Oct 22 '15 at 20:52
  • 28
    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. – vastlysuperiorman Jan 29 '16 at 22:09
  • 1
    I disagree, I prefer to write `if (result is None):` rather than `try: [. . .] except SimulationException as sim_exc: [. . .] else: ` – gregn3 Aug 26 '20 at 15:59
14

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.

S.Lott
  • 384,516
  • 81
  • 508
  • 779
  • I knew it was a bad idea, that's why I posted the question, by the look of it it's more of a code smell than I thought. thanks for the info – James Brooks Jan 07 '10 at 15:33
  • 3
    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. – Scott Griffiths Jan 08 '10 at 11:43
  • 6
    @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. – S.Lott Jan 08 '10 at 12:47
  • 100
    '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! – Rolf Bartstra Dec 06 '12 at 16:26
  • 13
    -1 This answer misleading and completely incorrect, since what @Rolf Bartstra says is true. Although in this case, what you say *can* be applied. – HelloGoodbye Jan 22 '14 at 19:18
  • There are sometimes good reason to test whether something is `True`, `False`, or `None`, for example, printing values from a database where `NULL` means "unknown status" and True/False mean Yes/No respectively. – Bruno Jul 14 '15 at 19:12
  • 5
    -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. – Samarth Shah Aug 23 '15 at 18:54
  • 1
    Regarding @RolfBartstra comment: Actually I see that as a good reason not to use `== True`. If there is a chance that `something` is not a boolean, what are we actually testing there? In Python 2 it is not even guaranteed that `1` will be equal to `True` (see [here](https://stackoverflow.com/q/2764017/2436175)). So I find S. Lott's suggestion absolutely wise. – Antonio Oct 12 '17 at 18:18
  • if something == True is still problematic, because (1.0 == True) == True, so this is not suitable to check if something has Value True – user__42 Sep 30 '20 at 07:58
  • 1
    What I found particularly useful about this answer is the advice to use " is None" instead of " == None". – Hamid Heydarian Oct 28 '20 at 21:04
  • What I would like to see from python, based on discussions like these, is when certain patterns are used like 'someBoolean == False' to give pythonic suggestions. – Joe Jul 26 '23 at 10:19
12

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.
Naeem Khoshnevis
  • 2,001
  • 4
  • 18
  • 30
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)
Thrastylon
  • 853
  • 7
  • 20
  • 3
    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.\ – user1767754 Jun 01 '18 at 07:03
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]
kgiannakakis
  • 103,016
  • 27
  • 158
  • 194