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 of decent quality or not by looking at it.
In this piece, we’ll look at some code smells of JavaScript classes, including lazy classes, excessive use of literals, too many conditionals and loops, orphaned variables and constants, and using too many variables instead of objects.
Lazy or Freeloader Class
A lazy or freeloader class is a class that does too little. If it doesn’t do much, it probably shouldn’t be added since it’s mostly useless.
We should find a way to put whatever is in the lazy class into a place that has more stuff. A class that has only one or two methods probably isn’t too useful.
Excessive Use of Literals
Using literals too much isn’t a good idea because repeating them will bring in more chances for errors. This is because we have to change each of them when we change code if there are too many of them.
Literals should be externalized into their own files and scripts. More dynamic data should be stored in databases where possible. This makes localization easy if needed.
For example, if we need to place the URL of Medium in multiple places in our code, we should have one constant with the URL rather than repeating it in multiple places.
So instead of:
const mediumRequest = fetch('[http://medium.com'](http://medium.com%27));
//...
const mediumJsUrl = '[https://medium.com/topic/javascript](https://medium.com/topic/javascript)';
We should instead write:
const MEDIUM_URL = '[http://medium.com'](http://medium.com%27);
const mediumRequest = fetch(MEDIUM_URL)
//.,.
const mediumJsUrl = `${MEDIUM_URL}/topic/javascript`;
This is better because we didn’t repeat [https://medium.com](https://medium.com/topic/javascript) in multiple places. Making changes then becomes easier.
Cyclomatic Complexity
Cyclomatic complexity means that there are too many conditional statements and loops in our code.
The complexity can arise in different ways. Loops and conditionals can be nested too deeply. More than two levels of nesting is probably too much and hard to read.
There can also be too many conditionals and loops that aren’t nested.
For example, instead of writing something like:
We should instead write:
We eliminated the nesting and moved some deeply nested code into its own function. This increases readability, and separating code into its own functions makes it easier to test.
Also, using loop control statements — like continue and break— in addition to return can help with controlling the flow of the code a lot without deeply nesting conditional statements with many lines inside.
Orphaned Variable or Constant Class
These are classes that have a collection of constants that belong elsewhere rather than in their own class.
This needs changing because it doesn’t make sense to put them in a place where they aren’t used. It’s not intuitive for anyone reading the code.
For example, if we have a class that has the following variable and we have the following classes:
Then the 'red' in the Color class is a better fit in the Apple class since we’re only using it for Apple instances.
So we can instead write:
class Apple {
constructor() {
this.color = 'red';
}
}
And not bother with having a Color class.
Data Clump
A data clump is a situation where we have too many variables passed around together in various parts of a program. This means that we should group these together into their own objects and pass them together.
For example, if we have a bunch of variables that we pass into a function as follows:
We should rewrite this so that all the variables are in an object instead and change the signature of the function to accept the object. As we can see, there are six parameters, which is too many. More than five is probably too many parameters for a function in most cases.
Also, we had to write out all the variables and they’re related, so we can group them into fields and reference the whole object instead of each variable separately.
This lowers the chance of missing variable references anywhere since grouping the variables into one object means that we only have to deal with one variable instead of six.
We also repeated the word fruit a lot.
We can eliminate all the repetition and reduce the number of variables and parameters we have to deal with by writing the following instead:
Now, we only have one object and parameter to deal with instead of several variables and parameters. It’s much easier on the eyes and more organized. Also, it’s harder for us to forget to reference some variables in our code since we have only one to deal with.
Conclusion
We shouldn’t have classes that do little or orphaned variables and constants. Also, too many loops and conditions — especially if they’re nested deeply — are hard to read and follow.
If we have many variables that are related to each other, they should be grouped into objects so that they’re easy to reference and deal with.
