“Code Smell” is Dead
“Code smell.” It’s a term that’s become increasingly popular in the world of software development. And the more I hear it, the more I dislike it. When has someone ever said, “that smells” and meant it in a good way?
What is a code smell anyway?
gives us a pretty straightforward definition for “code smell:”
“A code smell is a surface indication that usually corresponds to a deeper problem in the system.”
Not too bad, right? Well, if you dig a little deeper, things get worse. Take this title, for example (taken from the External Links in the "Code Smells" ):
If the concept of “code smell” has us casually comparing imperfect code to cancer… it’s time for a reset. Without a change in perspective, we risk doing some serious damage to our own teammates – and our profession at large.
In the People’s case against Code Smell, I offer exhibit A and B into evidence:
EXHIBIT A: Code Smell zaps team morale.
Evaluating code as categorically right or wrong does not help engineers perform their jobs any better. In fact, it may be the fastest way to zap team morale. I've seen this play out in code review more than anywhere else. I’m reminded of this tweet from :
When we focus our attention on correcting smelly code, we end up littering pull requests with critiques completely unrelated to the problem at hand.
However justified some comments may be, if a person is receiving too many critiques at once, their morale (and work) may suffer.
Every organization will have unique needs and values that determine their standards for code quality. However, what is non-negotiable is to place code quality over people or the task at hand.
On the other side of the coin, a simple task may suddenly become a very difficult pull request (PR) to review due to the amount of changes from a dev eliminating “smelly code”. It's hard enough keeping a PR small when doing feature work without extra diff for "clean up".
EXHIBIT B: The phrase doesn’t even make sense.
“Code smell” isn’t just pejorative – it’s misleading. Code smells are never an indication of failure. They’re patterns that will inevitably emerge over time – no matter how good of a developer you are. Every engineer on earth is going to find imperfect patterns in their code… because we’re finding solutions in an imperfect world. It’s often that we truly don’t understand what we wanted our code to be until after we’re done writing it.
Don’t get me wrong, I love writing clean code. We’re developers, after all. We’ve chosen this career because we’re makers and tinkerers – we enjoy elegant designs.
But at the end of the day, our role is to deliver value for our customers, our stakeholders, and our company… while supporting each other along the way. It’s not to write flawless code.
Maintaining a code base is difficult, no doubt. However, I feel it’s actually much less troublesome than maintaining good team morale and delivering great features. Even worse, I feel that if we get too caught up in maintaining standards or quality in our day to day, it leaves even less time for features, which in the end stresses the team, which leaves less time for maintenance and so on... a vicious cycle.
Enter, “Common Scents”
So, here’s a modest proposal. Let’s view imperfect patterns of code not as “code smell” but as “common scents.” This name is more accurate – and 100% more punny. Here’s why:
1) Mistakes in code are common.
If they weren’t common, we wouldn’t be able to identify code smells as a pattern in the first place. Hashtag meta.
2) These mistakes leave a trail for us to follow.
We should be able to analyze, trace, and track patterns of imperfect code – like a bloodhound on a (wait for it…) scent. Martin Fowler notes this as a “sniffable” quality.
It’s important to note that while we want to refresh and rethink the current approach to “common scents”, we aren’t saying these scents can’t cause problems. A code base full of undesirable patterns will lower productivity through every avenue available. This is due to the extraneous cognitive load this places on the programmer.
Speaking of cognitive load…..
Optimizing our cognitive loads
To understand cognitive load we will run through the 3 types.
1. Intrinsic cognitive load
This has to do with the inherent level of difficulty of a certain task. For example, basic addition is easier than calculus. And adding a button to existing UI is easier than adding a tooltip. (Anything’s easier than a tooltip.)
2. Germane cognitive load
This refers to the processing, construction, and automation of schemas. (You know, schemas – the patterns we create to organize all the data we encounter every day.)
Schemas are just cheat codes that help us organize data. And the better we increase our germane cognitive loads, the more efficient we’ll be.
3. Extraneous cognitive load
This is all about the way that information is presented – whether it’s documentation or the code itself. Examples can be:
- Poor formatting
- Heavy control structure
- Misleading variable names
- Outdated docs
- Large Pull Requests
You might experience an increased extraneous cognitive load if you're trying to read an important email but overhear your coworkers sharing Game of Thrones spoilers. There’s just too much going Oberyon.
For now, let’s focus on the latter two. Most of the time, common scents are caused by an unintended increase in extraneous (cognitive) load and/or an underutilized germane load.
Again, this is guaranteed to occur.
Time, requirements, or people are typically when common scents start to appear in our code. We can correct these issues by lowering our extraneous load, and trying to increase our germane load as much as possible.
1. Affirmative vs. Negative naming
We deal with Boolean values every day, but over time we may start to introduce complexity. Ideally I would want a strong correspondence of my variable names to the Boolean value it may hold.
We could prefer this example:
const isAvailable = room.status === 'available';
to this:
const isNotReserved = room.status === 'available';
It's also important to look at our code blocks to determine if our control structure is encouraging a negative naming convention for our Bools such as:
function reminder() {if(isNotAvailable) {return 'Meet in cafeteria';}return 'Meet in ' + room.name;}
It can be relatively easy to alter our logic and have to do a little less logic as we grok our codebase:
function reminder() {if(isAvailable) {return 'Meet in ' + room.name;}return 'Meet in cafeteria';}
2. Consistent returns
We think about this all the time as a tenant of functional programming - we should always receive the same output when given the same input. In javascript I extend this philosophy a little more agressively and if possible, return the same type for every method.
Let's take a simple function:
function getItems(id) {if (id) {return this.dataThing[id];}}
Depending on your linter, you may get flagged for an
So you return something ... kinda?
function getItems(id) {if (id) {return this.dataThing[id];}return; // leave me be!}
function getItems(id) {if (id) {return this.dataThing[id];}return null; // happy now?}
While this may be a contrived example, I'm sure many of us have run across similar issues in our day-to-day, resulting in fatal errors we find below.
const id = ``; // empty string as falsey inputconst { oxen, food, clothes } = getItems(id);// fatal error, can't destructure null or undefined ^^^
In my experience these kind of bugs usually result in borderline superstitious type checking such as:
const items = {};// double bang rules allif (!!getItems(id)) {items = { ...getItems(id) };}console.log(items.oxen);
Or bubblegum style short circuits, works in one spot but may fail in others:
const id = ``;const { oxen, food, clothes } = getItems(id) || {};console.log(oxen); // unexpected output, but no fatal error
Wouldn't it be easier to just have consistent returns?
/* returns Object */function getItems(id) {return id? this.dataThing[id]: {};}
3. Learn to fold.
If Marie Kondo can do it, so can you.
Lately, I've been working in heavily and I'm surprised at how few of my peers would take advantage of code folding. You cannot discount the heavy extraneous load caused by so many varying lines of code coming into your eyeballs.
It's a lot easier to find our makeMeatball function once we introduce code folding.
Wrapping up!
I don’t want anyone coming away from this post feeling ashamed for caring about code quality or enforcing standards. I only wish to ensure that we are caring for our teammates and delivering value first. And that when we do work on issues of code quality, we focus on eliminating the extraneous cognitive load in our codebases before we scrutinize our peers with textbook scruples.
If you have any questions please reach out via or email.
Special Thanks
Special thanks to these individuals for their help in formulating this post:
External Links
Additional reading, including sources of inspiration for this post
-
- timestamped at a relevant section on incremental feedback