Chapter 8
Code Smells and Refactoring

Code smells are indications that the code needs to be reorganized—a sign your software is undergoing code decay. Your code might need attention if you’re having thoughts like these:

• “I would never show this code during an interview.”
• “I’m going to start over and rewrite this code from scratch.”
• “Every time I look at this code, I have to re-figure-out what it does.”
• “These comments don’t match the code . . .”
• “Why is this code repeated in three different places?”
• “I want to switch out this component, but that’ll break X, Y, and Z in this other place, and I don’t want to deal with that.”

Types of codes smells we’ll cover (including how to fix them):

• Code smells about comments.
• Code smells about functions.
• General code smells (e.g., about the code within functions).

8.1 Why Care about Code Smells?

Reasons to pay attention to and fix code smells:

• Smelly code can be harder for you and others to maintain because the code is unclear. When code is hard to maintain, developers tend to work around it or re-create the same functionality elsewhere.
• Smelly code leads to smellier code. When you let your code become disorganized, you are giving yourself and others the message that smelly code is acceptable. Disorganized code also tends to give us an excuse to be lazy coders. A web development example: if you’ve used CSS, you may have encountered frustrating situations where the style you’re trying to apply is not working—somewhere in the code (e.g., other CSS, HTML, or JS), your style is being overridden. Instead of tracking down the competing code or markup, you use the “!important” property, which forces the style to be applied. The codebase is a mess anyway, so who cares? Your future self.
• Smelly code builds up technical debt. If the code is working, there’s never a reason to change it, right? Wrong. Each time you write sloppy code, you are contributing to your project’s technical debt. Maybe it works now, but as sloppy software grows, it will get more difficult to deal with. That can mean your company needing to hire more developers to keep productivity up. Instead, productivity can go down because now the old developers are struggling to teach the new developers, and everyone is continuing to write sloppy code (Martin, 2009). Ultimately, the software may have to be redeveloped entirely (which doesn’t always solve the problem). Or the project could fail.

8.2 Your Code Stinks—Now What?

If you can (e.g., your manager allows it), strongly consider refactoring. Refactoring is when you improve your code without changing what the code does. Refactoring is a way to pay down technical debt.

The remainder of this chapter is about code smells and how to clean them up. This is not an exhaustive list. You can find more advice in the references at the end of the chapter.

8.3 Comments

When we first learned to code, many of us didn’t write comments: solving problems and coding is fun; no time for boring comments! Then, we got more experience, started coding with others, were formally trained to code, or attempted to continue an old project, and we saw why comments are useful—and then some of us jumped to the other extreme: too many comments. We explained functions with paragraphs of prose, or even commented each line. It’s tedious, but it’s the right thing to do, right? Unfortunately (and fortunately), too many comments can be as bad as none.

8.3.1 Drawbacks of Having Many Comments

• Comments get out of date quickly. If we update the code, then procrastinate on the comments, what we leave can be misleading (to others and our future selves). Also, more comments mean greater likelihood some will be ignored, giving us the smelly situation of some accurate and some inaccurate comments. In that case, why would we trust any of the comments?
• Writing comments for straightforward code can distract from the important comments. If the code was difficult to write, is long, is unique, is complex, or has a “gotcha,” comments can help call attention to idiosyncrasies of the code.
• Writing lots of comments could indicate the code needs to be simplified. Ideally, most of the code you write will be self-explanatory, so frequent comments are not needed.

8.3.2 Code Smells about Comments

Below is a concise list of common code smells about comments and what to do about them (how to refactor).

• Obsolete Comment (no longer describes the code). Remove or update.
  1. # SMELLY
  2. """
  3. Uses the TwoFish block cipher with 256 bit key size
  4. """
  5. ThreeFish(512,data)
• Commented-Out Code (somebody thought they’d need that code later, but the commented-out block is now getting out of date and in the way). Remove. If you’re feeling risk-averse, save a backup or use a version-control system.
  Commenting out code often comes with poor assumptions (e.g., you’ll need the code later, others will understand why you commented it out, the surrounding code will continue having the same purpose, and so on).
  1. # SMELLY
  2. def updateWorldState():
  3.     """
  4.     updateTime() # might need later
  5.     updatePlayers()
  6.     updatePoints()
  7.     """
  8.     for p in players:
  9.         p.updateState()
• Redundant Comment (states what would already be immediately apparent to a programmer of any level). Remove. Less is more.
  1. # SMELLY
  2. getLength() # gets the length
• Long Comment (multiple sentences, complicated, goes into a lot of detail). Simplify the code to make it more self-explanatory; shorten or remove comment.
  1. # SMELLY
  2. """
  3. This is the first function I made in this module, and it takes the user’s Unicode text input, converts it to ASCII, then that creates a visualization of a typewriter typing the input. Problem is, as you might imagine, sometimes there’s no good conversion to ASCII, so some meaning is lost.
  4. """

8.4 Functions

A natural way to code is to start writing a function and then, as the program gets more complicated, keep adding to it. For example, if your program’s GUI only has a start and a stop button, the function for populating the screen with UI elements only needs to draw those two buttons. Then, when you add a menu and a settings button, you could update the function to draw those elements, too. You then add user accounts and decide that function is a fine place to check if the user is logged in, their level of inactivity, show a pop-up about cool new features . . . and your function balloons. Understanding the small details of how the function works can even make one feel proud—until the code becomes unmaintainable and bug-ridden.

8.4.1 Code Smells about Functions

Follow these refactoring suggestions to increase code readability, maintainability, and modularity.

• Long Function (more than 10 lines or so). Break into multiple functions. Aim for five lines or fewer.
• Function with Many Jobs (doing more than what its name suggests, doing things that aren’t closely related, doing many things). Break into multiple functions.
  1. # BEFORE
  2. def updateGUI():
  3.     updateTime()
  4.     updateTimeDisplay()
  5.     updateScores()
  6.     updateScoreDisplay()
  7.     refreshWindow()
  9. # AFTER
  10. def updateState() :
  11.     updateTime()
  12.     updateScores()
  14. def updateGUI():
  15.     updateTimeDisplay()
  16.     updateScoreDisplay()
  17.     refreshWindow()
• Function with Many Parameters (more than four, some say more than three). As appropriate, pass an object that combines the parameters, make calls within the function to get the parameter data, break into multiple functions, or find another way of reducing the number of parameters.
  Zero function parameters is even better than four!
  1. # BEFORE
  2. initOutdoorPlace(floraList, faunaList, temperature, windSpeed, cloudiness, rockiness, birdNoises, grassLength)
  4. # AFTER
  5. initOutdoorPlace(world1data)

8.5 Code

Code gets messy fast if you’re not paying attention. One reason is because many of us weren’t trained to be neat with code when we first learned it. To write tidy code, you may have to frequently stop and think about its design or be strict with yourself about refactoring regularly. Over time, you might adopt better habits.

8.5.1 Code Smells about Code in General

• Duplicate Code (same code in multiple places). Consolidate into one place, but watch out for creating unwanted dependencies.
  1. # BEFORE
  2. def updateLevelOfAlarm(npc):
  3.     if (npc.isWalking() && npc.isAlive() &&  npc.isFriendly())
  4.         setLevelOfAlarm(0)
  5.     else
  6.         setLevelOfAlarm(500)
  7.         react(npc)
  9. def react(npc):
  10.     if (npc.isWalking() && npc.isAlive() &&  npc.isFriendly())
  11.         keepWalking()
  12.     else
  13.         runAway()
  15. # AFTER
  16. def react(npc):
  17.     if (npc.isHarmless())
  18.         setLevelOfAlarm(0)
  19.         keepWalking()
  20.     else
  21.         setLevelOfAlarm(500)
  22.         runAway()
  24. def setLevelOfAlarm(level):
  25.     alarmLevel = level
  27. def isHarmless(npc):
  28.     return (npc.isWalking() && npc.isAlive() && npc.isFriendly())
• Long Lines (more than 100 characters or so). Shorten by breaking into multiple lines, converting to a function call, defining new variables, and so on.
  Thresholds like “100 characters” or “five lines” are arbitrary. Generally, shorter is better, but not even that rule can be applied everywhere. For example, “syntactic sugar” is the term for concise and elegant code syntax, usually built into the programming language. It can make your code shorter, but what’s the point if nobody can understand it!
  1. # BEFORE
  2. if (rectangle.coordinate[1][0] - rectangle.coordinate [2][0] > 500 && rectangle.coordinate[2][1] - rectangle.coordinate[3][1] > 500 && rectangle.isSquare()):
  4. # AFTER
  5. if (rectangle.isSquare() && rectangle.width > 500):
• Inconsistent Conventions (formatting code differently in different places, or untidily). Follow whatever style conventions the code is already using. If it’s a new project, plan to be self-consistent or follow accepted conventions for the language you’re using.
  When adding to another person’s code, it’s best to follow their coding style conventions even if you prefer a different way. If their code style is sloppy and inconsistent, however, consider whether there’s a polite way to fix the problem.
  1. # BEFORE
  2. if (whale.isSinging) {
  3.     activateAudioRecordingDevice();
  4. } else {
  5.     recording_device_off_confirmation_check();
  6. }
  8. if (starfish.blockingCamera)
  9. {
  10.     AirCannon.Spray(camera.coordinates);
  11. }
  13. # AFTER
  14. if (Whale.isSinging) {
  15.     activateAudioRecordingDevice();
  16. } else {
  17.     confirmRecordingDeviceOff();
  18. }
  20. if (Starfish.isBlockingCamera) {
  21. AirCannon.spray(Camera.coordinates);
  22. }
• Vague Naming (does not communicate what the function, variable, etc. is for). Rename it, even if the name is long. Long names can sometimes replace comments.
  Wouldn’t it be nice if code read like a book?
  1. # BEFORE
  2. a = 100
  3. b = 2
  5. # AFTER
  6. retail_price = 100
  7. wholesale_multiplier = 2

8.6 Summary

Cleaning up your code can help make your software sustainable and extensible and can make your teammates happier, too.

• Obsolete comment? Remove or update.
• Commented-out code? Remove.
• Redundant comment? Remove.
• Long comment? Simplify, shorten, or remove.
• Long function (more than ~10 lines)? Split.
• Function with many jobs? Split.
• Function with many parameters? Pass an object, make calls to get the parameter data, or split.
• Duplicate code? Consolidate.
• Long lines (more than ~100 characters)? Shorten, convert to function, or define new variables.
• Inconsistent conventions? Follow existing conventions.
• Vague naming? Rename.

References

Fowler, M., & Beck, K. (2019). Refactoring: Improving the design of existing code. Addison-Wesley.

Martin, R. C. (2009). Clean code: A handbook of Agile Software craftsmanship. Prentice Hall.

Shvets, A. (n.d.). Refactoring and Design Patterns. https://refactoring.guru/