Maxi Contieri

Posted on May 18, 2022 • Edited on May 20, 2022 • Originally published at maximilianocontieri.com

Code Smell 132 - Exception Try Too Broad

#python #programming #cleancode #webdev

Exceptions are handy. But should be as narrow as possible

TL;DR: Be as specific as possible when handling errors.

Problems

Fail fast principle violation
Missing errors
False negatives

Solutions

Narrow the exception handler as much as possible

Sample Code

Wrong

import calendar, datetime
try: 
    birthYear= input('Birth year:')
    birthMonth= input('Birth month:')
    birthDay= input('Birth day:')
    #we don't expect the above to fail
    print(datetime.date(int(birthYear), int(birthMonth), int(birthDay)))
except ValueError as e:
    if str(e) == 'month must be in 1..12': 
        print('Month ' + str(birthMonth) + ' is out of range. The month must be a number in 1...12')
    elif str(e) == 'year {0} is out of range'.format(birthYear): 
        print('Year ' + str(birthMonth) + ' is out of range. The year must be a number in ' + str(datetime.MINYEAR) + '...' + str(datetime.MAXYEAR))
    elif str(e) == 'day is out of range for month': 
        print('Day ' + str(birthDay) + ' is out of range. The day must be a number in 1...' + str(calendar.monthrange(birthYear, birthMonth)))

Right

import calendar, datetime

birthYear= input('Birth year:')
birthMonth= input('Birth month:')
birthDay= input('Birth day:')
# try scope should be narrow
try: 
    print(datetime.date(int(birthYear), int(birthMonth), int(birthDay)))
except ValueError as e:
    if str(e) == 'month must be in 1..12': 
        print('Month ' + str(birthMonth) + ' is out of range. The month must be a number in 1...12')
    elif str(e) == 'year {0} is out of range'.format(birthYear): 
        print('Year ' + str(birthMonth) + ' is out of range. The year must be a number in ' + str(datetime.MINYEAR) + '...' + str(datetime.MAXYEAR))
    elif str(e) == 'day is out of range for month': 
        print('Day ' + str(birthDay) + ' is out of range. The day must be a number in 1...' + str(calendar.monthrange(birthYear, birthMonth)))

Detection

[X] Manual

If we have a good enough test suite, we can perform mutation testing to narrow the exception scope as much as possible.

Conclusion

We must make exceptions as surgical as possible.

Relations

Code Smell 26 - Exceptions Polluting

Maxi Contieri ・ Nov 16 '20

#oop #exceptions #tutorial #programming

Credits

Photon from Jakob Braun on Unsplash

The primary duty of an exception handler is to get the error out of the lap of the programmer and into the surprised face of the user.

Verity Stob

Software Engineering Great Quotes

Maxi Contieri ・ Dec 28 '20

#codenewbie #programming #quotes #software

This article is part of the CodeSmell Series.

How to Find the Stinky parts of your Code

Maxi Contieri ・ May 21 '21

#codenewbie #tutorial #codequality #beginners

Top comments (5)

Michael Otu • May 18 '22

birthYear= int(input('Birth year:'))
birthMonth= int(input('Birth month:'))
birthDay= int(input('Birth day:'))

Wouldn't any of these also raise an error if the user enters a non-integer character?

Again in

print(datetime.date(int(birthYear), int(birthMonth), int(birthDay)))

the data above are been cast to int.

So I think you meant,

birthYear= input('Birth year:')
birthMonth= input('Birth month:')
birthDay= input('Birth day:')


try: 
    print(datetime.date(int(birthYear), int(birthMonth), int(birthDay)))
except ValueError as e:
   ....

Maxi Contieri • May 18 '22

Yes. You are right.
I am not very fluent in Python.
I try to use as many languages as possible to show code smells are language independent.
Thank you very much for the correction

xtofl • May 18 '22

I agree to the idea, but the example violates the 'fail fast' principle.

It should not let you input a month when the year is already unparseable.

Indeed, the very try block contains too much code, which results in an awkward effort to reconstruct which of the three statements went wrong.

This could be greatly improved by try/except for each input separately.

Shai Almog • May 18 '22

I'm not sure if I would agree with that as a general rule. The whole idea of exception handling is to propagate exceptions sometimes even way up the stack. For many cases I don't even catch exceptions and let them propagate into the generic handling code.

Maxi Contieri • May 18 '22

Yes.
This is for the case you want to catch it.
If you just want to propagate this code smell does not apply.

DEV Community