Subscribe to my email list now at http://jauyeung.net/subscribe/
Follow me on Twitter at https://twitter.com/AuMayeung
Many more articles at https://medium.com/@hohanga
Even more articles at http://thewebdev.info/
In programming, a code smell is a characteristic of a piece of code that indicates there may be deeper problems. It’s a subjective characteristic used for judgment of whether the code is decent quality or not by looking at it.
In this article, we’ll look at some code smells of JavaScript functions, including too many parameters, long methods, identifier length, returning too much data, and long lines of code.
Too Many Parameters
Functions can have too many parameters. If they have too many, it makes the function more complicated when reading it and calling it. It probably means that we can clean up the code in some way to make this easier to read.
For example, the following function takes many parameters:
const describeFruit = (color, name, size, price, numSeeds, type) => {
return `${fruitName} is ${fruitColor}. It's ${fruitSize}. It costs ${price}. It has ${numSeeds}. The type if ${type}`;
}
6 parameters are probably too many. We can clean this up by passing in an object instead:
const describeFruit = (fruit) => {
return `${fruit.name} is ${fruit.color}. It's ${fruit.size}. It costs ${fruit.price}. It has ${fruit.numSeeds}. The type if ${fruit.type}`;
}
As we can see, it’s much cleaner. We don’t have to worry about passing in many arguments.
It also fits better on the screen since it’s shorter.
5 parameters are probably the maximum that should be in a function.
Excessively Long Identifiers
Identifiers that are too long are hard to read. If they can be shorter without losing any information then make them shorter.
For example, we can shorten the following variable declaration:
let colorOfAFruit = 'red';
into:
let fruitColor = 'red';
by removing the Of and A to make it shorter. It doesn’t change the meaning or remove any information. However, it’s shorter so we type less and get the same results.
Excessively Short Identifiers
Identifiers that are too short are also a problem. This is because identifiers that are too short don’t capture all the meaning of the entity that we define.
For example, the following variable name is too short:
let x = 'red';
In the code above, x is too short since we have no idea what it means by looking at the variable name.
Instead, we should write:
let fruitColor = 'red';
So that we know the variable is the color of a fruit.
Excessive Return of Data
Functions that return data we don’t need isn’t good. A function should only return what’s needed by outside code so that we don’t expose extra stuff that isn’t needed.
For example, if we have the following function:
const getFruitColor = (fruit) => {
return {
...fruit,
size: 'big'
};
}
We have getFruitColor function with the size property, which isn’t relevant to the color of the fruit. Therefore, it isn’t needed and shouldn’t be returned with the object.
Instead, we should return a string with the fruit color as follows:
const getFruitColor = (fruit) => fruit.color;
The code above is much cleaner and only returns the fruit color as suggested by the name of the function.
Excessively Long Line of Code
Lines of code that are too long make the codebase hard to read, understand and debug.
Code that is so long that they don’t fit in the screen probably should be broken into multiple lines.
For example, instead of writing:
$("p").html("This is a paragraph").css("color", "red").css("text-align", "center").slideUp(1000).slideDown(1000);
We should instead write:
$("p")
.html("This is a paragraph")
.css("color", "red")
.css("text-align", "center")
.slideUp(1000)
.slideDown(1000);
This is much cleaner and doesn’t overflow the screen.
Each line of code shouldn’t be more 100 characters so that they can be read without scrolling on any screen.
Code formatters can rearrange the lines so that they’re shorter in most cases.
Conclusion
Having too many parameters in a method makes passing in data hard since it’s easy to miss some items. It also makes the method signature excessively long.
Identifiers should just be long enough to identify the information we need. Also, it shouldn’t be so short that we don’t get enough information from the identifier.
A function should only return the items that we need and no more. This makes using the function easy since there’s less data to handle and not expose extra information that we don’t want to expose.
Finally, long lines of code should be broken into multiple lines so that they’re easier to read and change. Code formatters can break code into multiple lines automatically.
Top comments (4)
Great article! For parameters, I would even argue 'rule of 3' applies here and then for expressiveness, destruct the object args inline, i.e.
describeFruit = ({ color, size, name }) =>. This will also let you assign defaults expressively (one place for someone to see where/if/what defaults assigned for missing args).Thanks.
Destructuring and default parameters are great features that we should use wherever we can.
It's also great that we only need one object parameter and the order doesn't matter.
Great Nice Good
Thanks