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 JavaScript, code smells still apply. Since there’re old constructs that should be eliminated, there’re additional issues that may arise from looking at old code.
In this article, we’ll look at some code smells using JavaScript code as examples. The code smells we look at include duplicate code, complex code, shotgun surgery, and classes that are too large.
Common Code Smells
Duplicate code
Duplicate code is code that is repeated in different places doing the same thing. Pieces of code that are very similar may also be considered duplicate.
This is a problem because we have to change multiple pieces of code when we have to change duplicate code. Also, it’s easy to forget that they’re there by other developers.
This is why the don’t repeat yourself (DRY) principle is emphasized a lot. It saves developers time.
For example, if we have
let numApples = 1;
in a code file, then we use that everywhere if we want to get the number of apples.
If we have repeated code that’s run many times, then we use a loop. If we want to run a snippet of code in multiple places but not repeated, then we can write a function and run that.
For instance, if we have something like:
let str = '';
str += 'foo';
str += 'bar';
str += 'baz';
We should instead write:
const arr = ['foo', 'bar', 'baz'];
let str = '';
for (let word of arr) {
str += word;
}
If we want code to be available in multiple modules, we export it and import it elsewhere.
For example, we can write the following in a JavaScript module called module.js and import the foo function elsewhere:
export foo = () => 'foo';
and call it as follows:
import { foo } from 'module';
const bar = foo();
Code That’s Too Complex
If there’s a simpler solution, we should go for that instead of writing something more complex.
Simple solutions are easy to read and change.
For example, if we have:
let str = '';
str += 'foo';
str += 'bar';
str += 'baz';
to create the string 'foobarbaz' , we can simplify it to:
let str = ['foo', 'bar', 'baz'].join('');
Shotgun Surgery
Shotgun surgery is a change that requires code to be multiple pieces of code to be changed.
This is a problem because changes are required in multiple places, which makes making the change difficult. It’s also easy to miss multiple places.
It’s usually caused by adding code that’s run in multiple places in the wrong place. There’s usually a better way to change code than to write code that’s used in multiple places.
For example, if we have an Express app and we want to add logging to it, we shouldn’t do the following:
const express = require('express');
const bodyParser = require('body-parser');
const app = express();
app.use(bodyParser.json());
app.use(bodyParser.urlencoded({ extended: true }));
app.get('/foo', (req, res) => {
console.log('foo called');
res.json({ message: 'foo' });
});
app.get('/bar', (req, res) => {
console.log('bar called');
res.json({ message: 'bar' });
});
app.get('/baz', (req, res) => {
console.log('baz called');
res.json({ message: 'baz' });
});
app.listen(3000, () => console.log('server started'));
The code above isn’t good because we have to add console.log to every route.
Instead, we should refactor the console.log into a route middleware that’s run before the code route code runs:
const express = require('express');
const bodyParser = require('body-parser');
const app = express();
app.use(bodyParser.json());
app.use(bodyParser.urlencoded({ extended: true }));
app.use((req, res, next) => {
if (req.path.substring(1)) {
console.log(`${req.path.substring(1)} called`);
}
next();
});
app.get('/foo', (req, res) => res.json({ message: 'foo' }));
app.get('/bar', (req, res) => res.json({ message: 'bar' }));
app.get('/baz', (req, res) => res.json({ message: 'baz' }));
app.listen(3000, () => console.log('server started'));
As we can see, the code above is much cleaner and it achieves the same thing as before, except that we only have one console.log instead of one for each route.
If we have more routes, then it’s going to be even harder to change the logging code. For example, if we want to replace console.log with some logging library, then it’s going be way harder if we put the logging code into every route.
Large Classes
Any class that’s large is going to be hard to maintain. Classes are supposed to create objects that only do one thing. We don’t want classes that are so big that they’re doing many things.
An object that’s too large is called a god object. A god object is an object that does too much. It’s very big and it does many things.
Each class is only supposed to solve small problems. If they’re large, then they’re probably doing too much.
Another issue that may arise from having classes that are too large is that different parts of the class depend on each other. The tight coupling creates lots of headaches when sorting out the code.
To solve this, we should the big class into smaller classes that does different things. Common functionality between them can be refactored into a parent class.
For example, if we have the following Employee class:
class Employee {
eat() {
//...
}
drink() {
//...
}
work() {
//...
}
sleep() {
///...
}
walk() {
//...
}
}
this is an example of a class that’s doing to much because it has methods that aren’t directly relevant for employees.
Instead, we can break the class into two classes as follows:
class Person {
eat() {
//...
}
drink() {
//...
}
sleep() {
///...
}
walk() {
//...
}
}
class Employee extends Person {
work() {
//...
}
}
Now the Employee only has the method that’s relevant to employees, and that’s the work method. We move the methods that are common to all people to the Person class so we can reuse them and declutter the Employee class.
It’s easy to clean up code and make it easy to read and change by following some basic principles. First, move duplicate code to a common location and reuse the code from the common location. Second, don’t create classes that do too many things. Third, if we need to make a code change in many places at once, there’s probably a better way to do it. Finally, don’t make code that’s too complex.
