4

Let's consider this example - code that returns index of the first None value in the list li (if present) or the last element's index if None is not present in li.

I could possibly check for None being in list but using ifs isn't the preferred way - Dive Into Python encourages to use exceptions as they are said to be faster and make code cleaner (see this question). So I came with three approaches:

1.

try:
    my_index = li.index(None)
except ValueError:
    my_index = len(li) - 1

print my_index

my_index is declared inside try/except block. Less lines, no redundant declaration. Possible problem - different exception type will result in not creating my_index and crashing the script on print.

2.

my_index = None  # or anything else
try:
    my_index = li.index(None)
except ValueError:
    my_index = len(li) - 1

print my_index

my_index is declared before try/except and has a value assigned regardless of what exception happens. Cons - more lines, looks redundant.

3. edit: Won't work - finally is executed regardless of try/except results

try:
    my_index = li.index(None)
except ValueError:
    my_index = len(li) - 1
finally:
    my_index = None  # or anything else

print my_index

Pros - always works (or at least I think so). Cons - I have never ever seen anyone using finally for my entire Python experience which must happen for a reason.

Which of these methods is considered the preferred and pythonic way? Maybe there is some better solution?

Artur
  • 733
  • 10
  • 25
  • 1
    I'd probably use **1.**. You said: "different exception type will result in not creating my_index", but if that happens then there's something drastically wrong with the code logic: `li` doesn't even exist, or it's an object that doesn't have an `.index` method, so you probably _want_ to crash at that point (or have some other error handling), you probably don't want to blithely continue on with a default `my_index` that's not really relevant. – PM 2Ring Aug 17 '17 at 10:54

4 Answers4

4
try:
    my_index = li.index(None)
except ValueError:
    my_index = len(li) - 1

print my_index

Possible problem - different exception type will result in not creating my_index and crashing the script on print.

Err, no. If any other exception is raised, this entire block of code will be aborted as the exception bubbles up to the caller. So if any other exception type besides ValueError is raised, print will never be executed. Which is a pretty sane behaviour: you handle the errors you expect and can handle, and let other errors cause the entire block to abort, possibly the entire script. This is the sanest and most concise solution for what you want.

deceze
  • 471,072
  • 76
  • 664
  • 811
  • Good point, I didn't test different exception types and I missed the fact, that other (unhandled) exception will stop execution at caller (thus `print` will not execute). Thanks! – Artur Aug 17 '17 at 11:33
1

The third option won't work. Check out the documentation on finally (emphasis mine):

A finally clause is always executed before leaving the try statement, whether an exception has occurred or not.

So in the third option, no matter what happens, my_index is always none.

That leaves us with the first and second option. It is worth noting that the second one is what you'll see in most statically typed languages. Python however is a bit more lenient and allows for either option.

Your argument regarding an exception that isn't ValueError is, I think, invalid. Another exception will halt execution and go back to the caller, so the print will never be reached. So the first solution should work just fine.

stybl
  • 8,580
  • 3
  • 24
  • 48
  • `except Exception: raise`…? That's pretty darn redundant. That outer `try` block says *"if any exception is raised, an exception is raised, else continue as usual."* Which means the entire outer `try` block is superfluous. – deceze Aug 17 '17 at 11:06
  • @deceze Looking at it again I realize that it would be entirely possible to move the `print` into the `try` clause, at which point we might as well get rid of it entirely. So yeah, redundant. – stybl Aug 17 '17 at 11:14
1

++ As @deceze noticed already, except ValueError: will catch only errors of this class and not others.

If you really need to process different error cases differently, you may provide more exception clauses:

try:
    ...
except ValueError:
    ...
except TypeError:
    ...

Sequences in except are also supported:

try:
    ...
except (ValueError, TypeError, ...):
    ...

+++ You should consider what does your code need to return if empty list was passed: -1? Raise an exception?

Violet Red
  • 201
  • 1
  • 4
  • Thanks! It works but is very unclear to read. Still, good point with different approach. – Artur Aug 17 '17 at 11:59
  • Apologies. I already considered this approach shitty and deleted the code ) Index() is really faster, though in more complex situations you may find generator expressions useful. – Violet Red Aug 17 '17 at 12:08
  • Sure. Thanks for sequences, never used them. – Artur Aug 17 '17 at 12:10
0

I think this is an example where you should not use an exception. While Python employs exception as "usual" programming constructs to reduce complexity, this is I think a case of adding complexity. Your code tells me that my_index has to hold a value after these lines and that you expect li to be an iterable, even under the occurrence of an exception. That this exception is ValueError which is the consequence of None not being in the list, is an added complexity, and one only understandable by convention - moreover, are you sure that its not IndexError that could be thrown? You see, IMHO the construct

if None in li: my_index = li.index(None)
else         : my_index = len(li) - 1

will not throw the programmer who comes across this code in half a year, i.e. maybe you yourself, off the track in understanding your program logic, while the version with the exception will cause additional mental work, raising the question if your anticipation of the possible exceptions and reactions was complete and correct. Only if this is a really really performance critical piece of code you should try to evade the double search.

Vroomfondel
  • 2,156
  • 1
  • 12
  • 24
  • 1
    In my opinion your first code sample is just as readable as try/except approach (although I'd rather stick close to pep8 formatting) but the second approach is _way_ less readable than try/except. Answering you question - this is not performance critical, readability is the key. – Artur Aug 17 '17 at 12:09
  • The second approach was buggy, sorry. But, sorry to say, to a programmer this should not be less readable. – Vroomfondel Aug 17 '17 at 12:10
  • @deceze yes, but if your read the answers and comments here, they circle about 80% around the correctness of the exception construct, so I keep my opinion on this one. – Vroomfondel Aug 17 '17 at 12:12