12

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.

4
  • 3
    YAGNI... You don't need it now and you can always add it later without too much difficulty. Commented Mar 8, 2015 at 15:56
  • Do you have any examples? Are you just raising Exception, for example, or more specific built-in errors?
    – jonrsharpe
    Commented Mar 8, 2015 at 17:50
  • just raising an Exception("specific description")
    – Ezra
    Commented Mar 8, 2015 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
    Commented Mar 8, 2015 at 22:35

1 Answer 1

9

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
  • 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
    Commented Mar 8, 2015 at 17:49
  • +1 for the simple statement "be consistent", with a team if you have one, with yourself if you don't. Commented Nov 15, 2017 at 18:04

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.