tags:

views:

385

answers:

4

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.

+9  A: 
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
The latter is kind of an explicit test against True or False, isn't it?
Peter Eisentraut
of course, but knowing that these are only possible return values, I don't think it's a problem.
SilentGhost
and it seem to be a bit faster as well
SilentGhost
A: 

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

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

try:
    result = simulate(open("myfile"))
except SimulationException:
    print "error parsing stream"
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.

Paul McGuire
Agreed. Much more pythonic than the evidently more popular solution above (which smells too much like C code).
Brandon Corfman
+2  A: 

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
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
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
@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