2

I have an exception instance and need to execute code depending on it's type. Which way is more clearly - re raise exception or isinstance check?

re raise:

try:
    raise exception
except OperationError as err:
    result = do_something1(err)
except (InvalidValue, InvalidContext) as err:
    result = do_something2(err)
except AnotherException as err:
    result = do_something3(err)
except:
    pass

isinstance check:

if isinstance(exception, OperationError):
    result = do_something1(err)
elif isinstance(exception, (InvalidValue, InvalidContext)):
    result = do_something2(err)
elif isinstance(exception, AnotherException):
    result = do_something3(err)

PS. Code is used in django process_exception middleware, therefore when re-raising exception I should write except:pass for all unknown exceptions.

  • 4
    You are not re-raising. You are using exception-specific handlers. What do you think is more readable and uses fewer levels of indentation? – Martijn Pieters Sep 04 '15 at 08:59
  • 5
    Also, don't use `except: pass`; you just silenced a `MemoryError` or `KeyboardInterrupt`. At most use `except Exception:` unless you have a very good reason (and usually, re-raise). – Martijn Pieters Sep 04 '15 at 09:00
  • Where does your exception come from in the first place? Can't you `try` ... `except` the source of the exception instead of capturing it in a variable? – 301_Moved_Permanently Sep 04 '15 at 09:02
  • 2
    This is mostly a matter of opinion. Depending on context both could be more clear. When I catch an exception I think it's more readable to do the check in the except-line (catching all and then using if-statements would lead to more complexity, more indentation and less clear code IMHO). – skyking Sep 04 '15 at 09:07
  • 1
    If you have so many options for things going wrong, then the code in question is doing too much, IMO. – mkrieger1 Sep 04 '15 at 09:23
  • It is all up to you and based on your preference. For performance, `isinstance` way is little bit slower since you have to first catch the exception with a general `except Exception as e` and run `if-else` under this. But beyond that, they all up to your flavour. – Mp0int Sep 04 '15 at 09:32

2 Answers2

1

You could store the exceptions you want to handle as keys in a dictionary with different functions as their values. Then you can catch all errors in just one except and call the dictionary to make sure the relevant function is run.

error_handler = {
                  OperationError: do_something1,
                  InvalidValue: do_something2,
                  InvalidContext: do_something2,
                  AnotherException: do_something3,
                }

try:
    #raise your exception
except (OperationError, InvalidValue, InvalidContext, AnotherException) as err:
    result = error_handler[type(err)]()

I suspect there might be a way to programmatically pass error_handler.keys() to except, but the means I've tried in Python2.7 have not worked so far.

Note that as martineau points out, because this uses type(err) as a dictionary key it won't handle derived exception classes the way that isinstance(err, ...) and except (err) would. You'd need to match exact exceptions.

SuperBiasedMan
  • 9,814
  • 10
  • 45
  • 73
  • Thank you for this idea, but i wrote functions `do_something1, 2,3` just for example. In my case there might be random code block. – Фархад Хатамов Sep 04 '15 at 10:06
  • @ФархадХатамов I thought that might be the case, but I still think you could consider making functions to run here as there's not much downside unless you'd have scoping issues or have to pass all sorts of different parameters. – SuperBiasedMan Sep 04 '15 at 10:22
1

First get rid of the except: pass clause - one should never silently pass exceptions, specially in a bare except clause (one should never use a bare except clause anyway).

This being said, the "best" way really depends on concrete use case. In your above example you clearly have different handlers for different exceptions / exceptions sets, so the obvious solution is the first one. Sometimes you do have some code that's common to all or most of the handlers and some code that's specific to one exception or exceptions subset, then you may want to use isinstance for the specific part, ie:

try:
   something_that_may_fail()
except (SomeException, SomeOtherException, YetAnotherOne) as e:
   do_something_anyway(e)
   if isinstance(e, YetAnotherOne):
      do_something_specific_to(e)

Now as mkrieger commented, having three or more exceptions to handle may be a code or design smell - the part in the try block is possibly doing too many things - but then again sometimes you don't have much choice (call to a builtin or third-part function that can fail in many different ways...).

bruno desthuilliers
  • 75,974
  • 6
  • 88
  • 118