Code smells are signs that something is wrong with your code and demands your attention. By investigating the smell, you can find and (hopefully) fix its underlying cause, improving your code in the process. In this post, we want to help you write better JavaScript, not via tools, but by following some best practices. In this post, we want to analyze JavaScript code smells.
This post has a pretty straightforward structure: it starts by quickly defining JavaScript code smells (and smells in general) with a little more depth. Then, it proceeds to cover the smells themselves, with explanation and, when applicable, code examples. Some of the smells are JavaScript specific, while others apply to any language. Be aware of all of them when writing JavaScript, and your code will change for the best.
Roll up your sleeves, prepare your nose, and let’s get started!
Defining JavaScript Code Smells
JavaScript code smells are code smells that can affect JavaScript code. But what are code smells?
I’ve first learned about code smells by reading a post on Coding Horror. In the post, Jeff Atwood calls code smells “warning signs in your own code.” That’s not that different from how Wikipedia defines them:
In computer programming, a code smell is any characteristic in the source code of a program that possibly indicates a deeper problem.
The important thing to keep in mind about code smells is that they’re not necessarily a problem. In other words, code smells are not synonymous with anti-patterns. Instead, they are signs that something might be wrong with your code.
So, it’s more productive to consider code smells not as problems that need to be eliminated, but rather as prompts for further investigation.
JavaScript Code Smells: Here Are 7 to Be Aware Of
We’re done defining JavaScript code smells. Without further delay, let’s go through our list of smells, explaining why they might be symptoms of deeper problems and what you should do about them if anything.
Too Many Indentation Levels
We start our list with a smell that applies to virtually all programming languages: too many indentation levels. What exactly do we mean by that, and why is it a problem?
Let’s begin by taking a look at the following code sample:
function add(numbers) {
var result = 0;
var parts = numbers.split(',');
for (var i = 0; i < parts.length; i++) {
var integer = parseInt(parts[i]);
if (!isNaN(integer)) {
if (integer >= 0) {
if (integer <= 1000) {
result += integer;
}
}
}
}
return result;
}
The function above is inspired by the famous String Calculator Kata by Roy Osherove. The goal is to create a function that receives a string containing a list of numbers separated by a comma and then calculates their sum. The rules say that the function should ignore numbers higher than 1000, and throw an error/exception if one or more negative numbers are passed. For simplicity’s sake, my function just ignores negative numbers.
So, as you can see, the code above contains “a” for structure and, inside it, three nested “ifs.” Sure, it’s just a simple example, but think of it as a proxy for more complex code. Imagine that, at the deepest level (inside the innermost “if”) we had, instead of a single line, 50. Go further and imagine that our code is not four levels deep, but eight or nine. Code like this exists in real life, making it harder for developers to read it and reason about it. When you’re several levels deep, it becomes harder and harder to reason about the code, keeping track of variables’ values and results of conditions. Also, code with too many levels stretches horizontally, making it hard to read on mobile devices, on smaller screens, and also when splitting screens (when performing a code review, for instance.)
OK, but how much is too much? How many levels of indentation should you strive for? That might be somewhat of a subjective matter—not entirely, as you’ll see soon. As a rule of thumb, you can adopt three as the maximum allowed and then work from there, tweaking and experimenting until you find the right number for your project and team.
Long Functions
The second item in the list is closely related to the previous one, and it makes sense. Functions with too many levels of indentation are likely long, and long functions are also somewhat likely to have many levels. But what would the problem with long functions be?
All else being equal, short functions are just easier to deal with. They’re easy to read since there isn’t a lot to read. As a consequence, they’re easier to troubleshoot, since there’s isn’t a lot of code in which a bug can be hiding.
Then again: how long is “long?” That’s going to depend on several factors, including the language. Since we’re talking about JavaScript here, which is a dynamic language, the “proper” number will likely be less than it would be for a static language such as Java. Since many people recommend 20 lines as a useful method size for Java, let’s use half of that. Start with ten lines and change that as you see fit.
Too Many Parameters
The trend continues with yet another excess related code smell. Here we’re talking about function parameters. Too many of them are also a bad sign, and the reasons are pretty much the same as in the previous items. Functions with too many parameters are harder to read, understand, and troubleshoot. Besides, functions with more parameters are more likely to be longer and more complex.
Speaking of complex…
Code That Is Too Complex
The previous three items have all something to do with “excess,” but they have a degree of subjectivity. This item, on the other hand, is as objective as it can get, since it involves an actual metric.
We’re talking about cyclomatic complexity, which was developed in 1976 by Thomas J. McCabe, Sr. It refers to the number of possible independent paths a function can take.
A piece of code with high cyclomatic complexity is harder to reason about and troubleshoot. Also, it might make testing harder, since it increases the number of minimum test cases you’d need to test the function.
Copy Pasted Code
It should stand to reason that copying and pasting code, then changing it a little bit, isn’t a good development practice, even if you don’t know the term code smell. So, not really a lot to say here, except “don’t copy and paste code” blindly, without understanding what it does and how it works. Doing so is an example of Cargo Cult Programming, which basically means doing things without understanding them.
Wrong Use of Equality
Performing equality comparisons in JavaScript can be tricky, especially for those who come from other languages. That’s because the language features both the “==” and “===” operators. What happens is that many developers use the version with the two equal signs in situations when they should use the other. Consider the line of code below:
1 == ' 1'
The result of that comparison is true, which might seem odd for developers who aren’t used to JavaScript. To understand why that happens, you must first bear in mind that JavaScript features strict and type–converting comparisons. A strict comparison is true when the operands have the same type, and the values are equal. The type–converting comparison, as the name makes clear, converts the operands to the same type before making the comparison.
Now it’s easier to understand what happens in the line above: the equality operator (==) performs a type-converting comparison. To perform a strict comparison, you should use the identity operator (===) .
Comments / Dead Code
It might surprise you to see “comments” as an item in our code smell list, but it sure is. Comments might be harmless, but often they’re not. They frequently get out of sync with the code they’re supposed to document. That renders them not only useless but harmful since lying documentation is worse than no documentation at all.
Comments are also often used to explain a piece of code that is too complex. So, instead of commenting the code, you should strive to refactor into in order to make it easier to understand.
Finally, comments are also often used to “deactivate” a part of the code. Since we all use version control nowadays—right? right!?—there is no excuse for doing that. Mercilessly delete dead code from your codebase, and don’t be sorry about it.
Where to Go Now?
In this post, we’ve covered seven code smells that might affect your JavaScript codebases. The first step is to be aware of them, and we’ve just helped with that. What are the next steps?
In some scenarios, there might be no next steps. As we’ve explained, a code smell is not necessarily a bad thing. Rather, it’s a sign of a potentially harmful thing that demands your attention. So, even though long functions are generally a bad sign, your particular project might have legitimate reasons for having some long functions. And the same applies for most smells you’ve seen today.
But there are times when there are next steps available, and there are tools that can help you there. For detecting copied and pasted code, you can use tools like jsinspect and jscpd. For most of the other smells, you should definitely employ a linter, which can help you automate many types of code quality checks.
After detecting the problematic signs and deciding that they need fixing, the next step is obviously fixing them, and you do that by employing the sort of arch-nemesis of code smells, which is refactoring.
Thanks for reading!
This post was written by Carlos Schults. Carlos is a .NET software developer with experience in both desktop and web development, and he’s now trying his hand at mobile. He has a passion for writing clean and concise code, and he’s interested in practices that help you improve app health, such as code review, automated testing, and continuous build.
