4.9. Code Smell Myths¶
Some code smells aren’t really code smells at all. Programming is full of half-remembered bits of bad advice that are taken out of context or stick around long after they’ve outlived their usefulness. I blame tech book authors who try to pass off their subjective opinions as best practices.
You might have been told some of these practices are code smells, but they’re mostly fine. I call them code smell myths: they’re warnings that you can and should ignore. Let’s look at a few of them. ## Myth: Functions Should Have Only One return Statement at the End The “one entry, one exit” idea comes from misinterpreted advice from the days of programming in assembly and FORTRAN languages. These lan- guages allowed you to enter a subroutine (a structure similar to a function) at any point, including in the middle of it, making it hard to debug which parts of the subroutine had been executed. Functions don’t have this prob- lem (execution always begins at the start of the function). But the advice lingered, becoming “functions and methods should only have one return statement, which should be at the end of the function or method.”
Trying to achieve a single return statement per function or method often requires a convoluted series of if - else statements that’s far more con- fusing than having multiple return statements. Having more than one return statement in a function or method is fine.
4.9.1. Myth: Functions Should Have at Most One try Statement¶
“Functions and methods should do one thing” is good advice in general. But taking this to mean that exception handling should occur in a separate function goes too far. For example, let’s look at a function that indicates whether a file we want to delete is already nonexistent:
>>> import os
>>> def deleteWithConfirmation(filename):
>>> try:
>>>
>>> if (input('Delete ' + filename + ', are you sure? Y/N') == 'Y'):
>>>
>>> os.unlink(filename)
>>>
>>> except FileNotFoundError:
>>>
>>> print('That file already did not exist.')
Proponents of this code smell myth argue that because functions should always do just one thing, and error handling is one thing, we should split this function into two functions. They argue that if you use a try - except statement, it should be the first statement in a function and envelop all of the function’s code to look like this:
>>> import os
>>> def handleErrorForDeleteWithConfirmation(filename):
>>>
>>> try:
>>>
>>> _deleteWithConfirmation(filename)
>>>
>>> except FileNotFoundError:
>>>
>>> print('That file already did not exist.')
>>>
>>> def _deleteWithConfirmation(filename):
>>>
>>> if (input('Delete ' + filename + ', are you sure? Y/N') == 'Y'):
>>>
>>> os.unlink(filename)
This is unnecessarily complicated code. The deleteWithConfirmation() function is now marked as private with the underscore prefix to clarify that it should never be called directly, only indirectly through a call to handleErrorForDeleteWithConfirmation() . This new function’s name is awkward, because we call it intending to delete a file, not handle an error to delete a file.
Your functions should be small and simple, but this doesn’t mean they should always be limited to doing “one thing” (however you define that). It’s fine if your functions have more than one try - except statement and the state- ments don’t envelop all of the function’s code. ## Myth: Flag Arguments Are Bad Boolean arguments to function or method calls are sometimes referred to as flag arguments. In programming, a flag is a value that indicates a binary set- ting, such as “enabled” or “disabled,” and it’s often represented by a Boolean value. We can describe these settings as set (that is, True ) or cleared (that is, False ).
The false belief that flag arguments to function calls are bad is based on the claim that, depending on the flag value, the function does two entirely different things, such as in the following example:
- def someFunction(flagArgument):
if flagArgument:
- # Run some code...
else:
# Run some completely different code...
Indeed, if your function looks like this, you should create two sepa- rate functions rather than making an argument decide which half of the function’s code to run. But most functions with flag arguments don’t do this. For example, you can pass a Boolean value for the sorted() function’s reverse keyword argument to determine the sort order. This code wouldn’t be improved by splitting the function into two functions named sorted() and reverseSorted() (while also duplicating the amount of documenta- tion required). So the idea that flag arguments are always bad is a code smell myth.
##Myth: Global Variables Are Bad
Functions and methods are like mini-programs within your program: they contain code, including local variables that are forgotten when the function returns. This is similar to how a program’s variables are forgotten after it terminates. Functions are isolated: their code either performs correctly or has a bug depending on the arguments passed when they’re called.
But functions and methods that use global variables lose some of this helpful isolation. Every global variable you use in a function effectively becomes another input to the function, just like the arguments. More arguments mean more complexity, which in turn means a higher likeli- hood for bugs. If a bug manifests in a function due to a bad value in a global variable, that bad value could have been set anywhere in the pro- gram. To search for a likely cause of this bad value, you can’t just analyze the code in the function or the line of code calling the function; you must look at the entire program’s code. For this reason, you should limit your use of global variables.
For example, let’s look at the calculateSlicesPerGuest() function in a fic- tional partyPlanner.py program that is thousands of lines long. I’ve included line numbers to give you a sense of the program’s size:
>>> def calculateSlicesPerGuest(numberOfCakeSlices):
>>>
>>> global numberOfPartyGuests
>>>
>>> return numberOfCakeSlices / numberOfPartyGuests
Let’s say when we run this program, we encounter the following exception:
- Traceback (most recent call last):
- File "partyPlanner.py", line 1898, in <module>
print(calculateSlicesPerGuest(42))
- File "partyPlanner.py", line 1506, in calculateSlicesPerGuest
return numberOfCakeSlices / numberOfPartyGuests
ZeroDivisionError: division by zero
The program has a zero divide error, caused by the line return numberOfCakeSlices / numberOfPartyGuests . The numberOfPartyGuests variable must be set to 0 to have caused this, but where did numberOfPartyGuests get assigned this value? Because it’s a global variable, it could have happened anywhere in the thousands of lines in this program! From the traceback information, we know that calculateSlicesPerGuest() was called on line 1898 of our fictional program. If we looked at line 1898, we could find out what argument was passed for the numberOfCakeSlices parameter. But the numberOfPartyGuests global variable could have been set at any time before the function call.
Note that global constants aren’t considered poor programming prac- tice. Because their values never change, they don’t introduce complex- ity into the code the way other global variables do. When programmers mention that “global variables are bad,” they aren’t referring to constant variables.
Global variables broaden the amount of debugging needed to find where an exception-causing value could have been set. This makes abun- dant use of global variables a bad idea. But the idea that all global variables are bad is a code smell myth. Global variables can be useful in smaller pro- grams or for keeping track of settings that apply across the entire program. If you can avoid using a global variable, that’s a sign you probably should avoid using one. But “global variables are bad” is an oversimplified opinion.
4.9.2. Myth: Comments Are Unnecessary¶
Bad comments are indeed worse than no comments at all. A comment with outdated or misleading information creates more work for the programmer instead of a better understanding. But this potential problem is sometimes used to proclaim that all comments are bad. The argument asserts that every comment should be replaced with more readable code, to the point that programs should have no comments at all.
Comments are written in English (or whatever language the program- mer speaks), allowing them to convey information to a degree that variable, function, and class names cannot. But writing concise, effective comments is hard. Comments, like code, require rewrites and multiple iterations to get right. We understand the code we write immediately after writing it, so writ- ing comments seems like pointless extra work. As a result, programmers are primed to accept the “comments are unnecessary” viewpoint.
The far more common experience is that programs have too few or no comments rather than too many or misleading comments. Rejecting com- ments is like saying, “Flying across the Atlantic Ocean in a passenger jet is only 99.999991 percent safe, so I’m going to swim across it instead.”
Chapter 10 has more information about how to write effective comments.