10

In my code there are about seven places where I raise an exception. All of these exceptions are treated the same: print an error to log file, return software state to default and exit.

During code review my senior engineer, whom I value a lot, said I should subclass all of these exceptions. His argument is that in the future we may want to handle the exceptions differently and that will be easier.

My argument is that currently it will only clutter our code and, since we don't know whether we will ever handle the exceptions differently, we should leave the code terse and, if and when the time comes, then and only then we should subtype.

I would like to hear any argument for each case.

  • 2
    YAGNI... You don't need it now and you can always add it later without too much difficulty. – Robert Harvey Mar 8 '15 at 15:56
  • Do you have any examples? Are you just raising Exception, for example, or more specific built-in errors? – jonrsharpe Mar 8 '15 at 17:50
  • just raising an Exception("specific description") – Ezra Mar 8 '15 at 18:10
  • @Ezra at the least, you should see if there's a more fitting built-in exception (see docs.python.org/2/library/exceptions.html). – jonrsharpe Mar 8 '15 at 22:35
8

You're right

The argument for your side is already mentioned by Robert Harvey: don't add code you don't need right now, especially since it's easy to add it later.

Your reviewer is right, too

On the other hand, the reviewer's point is understandable as well:

  • Returning a generic Exception() is not very helpful to the caller: while the exception description indicates to an human what's going on, treating exceptions differently programmatically may be impossible. The developer using your code may be reluctant changing the types of exceptions, including by fear (justified or not) of breaking something.

    Note that adding custom exceptions right now is not that difficult:

    class MyCustomException(Exception):
        pass
    

    is all you need. That's only two lines of code (given that you may not even need to create a separate file if you put custom exceptions in one file).

  • The code itself looks better, more readable.

    if price < self.PriceMinValue:
        raise OutOfRangeException("The price is inferior to zero.")
    

    looks slightly more readable compared to:

    if price < self.PriceMinValue:
        raise Exception("The price is inferior to zero.")
    

    because of the indication of the type of exception:

    • In the second piece of code, I need to read the description and guess that the price is out of range (or maybe it's not? Maybe there are cases where price can be negative, such as rebates?)

    • In the first piece of code, a glimpse over the type gives an immediate indication about the error. It looks like there is a set of allowed values for a price, and the current value is outside this set.

So?

So:

  • Both approaches are valid. If you don't subclass the exceptions when you don't need custom types, you're right. When you do subclass the exceptions because it costs nothing to do it and may be useful later, you're right to.

  • Be consistent with your team. If your team uses custom exceptions extensively, use them.

  • 2
    But there is a happy medium: raise ValueError('The price is less than zero'). This is more specific than the base Exception, but without any fuss. – jonrsharpe Mar 8 '15 at 17:49
  • +1 for the simple statement "be consistent", with a team if you have one, with yourself if you don't. – Styne666 Nov 15 '17 at 18:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.