What Is It That Makes Your Code Smell?
Eight code smells listed, so you can avoid them
Probably most code that you’ve seen smells a little bit. And worst case, it really stinks. Chances are you have introduced some code smells yourself. But what exactly is a code smell?
A code smell is a surface indication that there might be a problem regarding your system and the quality of your code. Most of the time, code smells require some kind of refactoring to be fixed. Even though smelly code works, it is an indicator that the code is of low quality.
In this article, we’ll be going over a list of eight common code smells so you can avoid them.
“The best smells are something that’s easy to spot and most of the time leads you to really interesting problems” — Martin Fowler
1. Shotgun Surgery
Have you ever stumbled upon the situation where a small change in some class leads to you having to change many different classes? Well, shotgun surgery it is.
Personally, shotgun surgery is one of my “favorite” code smells. I’ve seen many developers introduce this code smell in their code. Shotgun surgery inevitably leads to lots of duplicate code. Thus, the concerns with this style are largely the same as those for any duplication in code.
Duplicating the same logic in many places can vastly increase the costs of making changes to the same logic later. This will lead to increased development time. Small features will take more time to develop since the code base gets more and more unmaintainable.
2. Oddball Solutions
An oddball solution is when a problem is solved one way throughout a system and the same problem is solved another way in the same system.
One of these solutions is the oddball, or inconsistent, solution. The presence of this smell usually indicates subtly duplicated code.
To get rid of the oddball solution you should start by determining your preferred solution. In some cases, the solution used least often may be your preferred solution if it is better than the solution that’s used more often.
After determining your preferred solution, you can start removing the oddball solution from your code and replace it with the preferred solution.
Once you’ve implemented the preferred solution consistent throughout your entire codebase, you may be able to move all instances of this solution to one place, thereby removing duplication.
3. Overcomplicating the Obvious
We’ve all seen code where design patterns were introduced just for the heck of it. Especially junior developers tend to do this. This is their way of showing off and letting the other developers know how good they can code. Little do they know, it reduces the quality of the code.
But when exactly can we say that someone is overcomplicating things? A good rule of thumb is when you’re writing code that solves problems you don’t have.
The worst kind of overcomplicating is usually committed in the name of making code future-proof — which achieves the exact opposite.
4. Too Many Parameters
Robert C. Martin said the following thing about parameters in his book Clean Code: A Handbook of Agile Software Craftsmanship:
“The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible. More than three (polyadic) requires very special justification — and then shouldn’t be used anyway.
I think this sums it up pretty well. Use as few parameters as possible.
If you find yourself passing a group of data received from another object as parameters, you should consider passing the object itself to the method.
Once you reduce the number of parameters used in methods, you’ll get more readable and shorter code.
5. Long Methods
We’ve all seen methods that take up more than a hundred lines of code. All faith in the quality of this code is long gone.
The reason that this code smell gets introduced is very simple. Most of the time, code gets added to a method but nothing is ever taken out. Since it is easier to write code than to read it, this code smell remains unnoticed until the method has become ugly, and oversized.
Long methods tend to have more than one responsibility. It is considered best practice to follow the single responsibility pattern. This means that a method can only do one thing, and one thing only.
6. Inconsistent Naming
To ensure code readability and keep your code maintainable, naming in your code must be done properly.
I wrote a post a while ago about some simple rules that you can follow to create meaningful names in your code. And I can’t emphasize enough how important good variable names are. Choosing good names takes time, but it saves more time than it takes.
The problem with naming is that while you’re developing you’re making the naming decisions, but that is the time when those decisions are the least impactful.
Everything is fresh in your mind. You know what to build, you know the architecture and whatever weird feature you’re coding makes sense.
However, whenever the project is delivered it enters the maintenance phase. This is the longest phase by far, and this is where naming things becomes important.
There are a lot of different ways that you can use for naming, each having its pros and cons. What’s most important is that these conventions need to be agreed upon and you should stick with them.
7. Data Clumps
Sometimes, you just find out that a small set of parameters is constantly passed around together across different sections of your codebase. Those distinct values are referred to as data clumps.
If multiple method calls take the same set of parameters, it may be a sign that those parameters are related. To keep the group of parameters together, it can be useful to combine them in a new object. This will improve the organization of your codebase.
Having a data clump doesn’t have to be a problem, but it might be an indicator of a deeper problem.
8. Middle Man
Sometimes you have a class that has many methods that do nothing but delegate to another method in a different class. In that case, the class that is delegating is considered a middle-man class.
There are some use cases where having a middle-man class is considered helpful. When building a facade, for example. But most of the time it would be better to get rid of the middle-man class.
To get rid of this code smell, you should try to communicate directly with the class that implements the behavior rather than going through the middle man.