5 most common code smells that you should avoid
Tight deadlines, lack of experience, miss code review… These are just some of the reasons that can lead us to write poorly planned code. The consequence of this fact is that we end up making mistakes during development, creating the so-called code smell.
A code smell is any characteristic in code that possibly indicates a deeper problem. Determining what is and isn’t a code smell is subjective, and varies by language and development methodology.
In this article we’ll talk about the five most common code smells that you should avoid, in my opinion. I’ll explain each one, give examples, and a way to refactor that smell. Of course, there are other ways to refactor each smell, you can learn other ways from the references I left at the end of this article.
👬 1 - Code Duplication
This code smell occurs when there are identical or very similar pieces of code in more than one location. There’s also more subtle duplication when specific parts of code look different but perform the same job.
In the example below, we have a script that averages two arrays of numbers and displays a message based on the result. Note that we have code duplication in the averaging and in the result analysis:
Solution
The fix for this smell is generally simple. We just need to extract the duplicate code to a separate method and call it everywhere that the code is duplicated.
In our example, we can remove the duplicate code by extracting two news methods. The first is responsible for calculating the average of the numbers in the array. The second is responsible for analyzing the average and display the result.
We could create a single method that would be responsible for calculating the average and analyze it, but it would break the Single Responsibility Principle described in SOLID
Yet, there are some edge cases when isn’t possible to extract a method, like when the duplicate code is similar but not completely identical. In these cases, we need to use specific techniques for each situation. I’ll explain each case in a future article soon.
For now, note in the next few topics that all code smell solutions help us remove code duplication throughout our code
Benefits
Merging duplicate code simplifies the structure of your code and makes it shorter, in other words, the code becomes easier to simplify and more maintainable.
By removing code duplication, we also remove possible inconsistencies and business rule failures in the code, since we centralize all the business logic of the refactored feature in a single point of the application.
📏 2 - Long Method
A method contains too many lines of code. As a general rule, any method longer than ten lines should be re-evaluated.
Usually, something is always being added to a method but nothing is taken out. Thus, this smell remains unnoticed until the method turns into an ugly, oversized beast
As a rule of thumb, if you feel the need to comment on something inside a method, you should take this code and put it in a new method. Give the new method a descriptive name and your code will be cleaner.
Also, a long method usually breaks the Single Responsabilty Principle from SOLID and can result in code duplication
In the example below, we have a class User that has a method printDetails. Note that this method has multiple responsibilities: calculate the user’s age, form the user’s full name, and display them. This ends up resulting in a method with too many lines of code.
Solution
Just like the previous code smell, we can just extract new methods from the refactoring target method. Don’t forget to give the new methods descriptive names to make the code more readable.
In our example, we just need to move the age calculation and the full name formation responsibilities to two new methods:
Benefits
Keep in mind: classes with short methods live longer. The longer a method or function is, the harder it becomes to maintain it.
Also, short methods help us to avoid code duplication throughout our code and allow code reuse.
🤖 3 - Large Class
Just like methods, classes usually start small. But over time, they get bloated as the program grows. This code smell definition is clear: a class contains many fields/methods/lines of code/responsibilities, in other words, it’s doing too much.
Large classes, such as long methods as well, indicates weaknesses in design that can possibly slow down the development or increase the chance of failures in the future. In addition, it makes the system more difficult to understand, read and develop.
Moreover, a large class breaks the Single Responsabilty Principle from SOLID and can result in code duplication
In the example below, we have a class Student with multiple properties: name, lastName, age, pencil, pen, and ruler. The problem here is that maybe the class has more responsibilities than it should.
This conclusion is questionable, but what if we had another class called Teacher that has pencil, pen, and ruler too? In this case, we’d need to duplicate all the Student class logic to the new class.
Solution
To fix this code smell, we have to identify all the responsibilities of the large class. Then, for each responsibility we extract out a new class, moving the related fields and methods.
In our example, we can extract the materials-related properties from the Student class to a new class Material. This way, we decrease the Student class size and, as a bonus, we create a new component Material that can be reused in future implementations without code duplication.
Benefits
The greatest benefit of refactoring large classes is avoiding code and functionality duplication.
📜 4 - Long Parameter List
A long parameter list in a method call is a code smell. It indicates that there might be something wrong with the implementation. It might happen after several algorithms are merged in a single method.
There is no single rule for how many is too many parameters. Usually, more than three or four is considered too many.
In the example below, we have a User class that receives too many parameters in the constructor. The problem is that is hard to understand that long list, which becomes hard to use as it grows longer.
Instead of a long list of parameters, a method can use the data of its own object. If the current object doesn’t contain all necessary data, another object can be passed as a method parameter.
Solution
Instead of a long list of parameters, a method can use the data of its own object. If the current object doesn’t contain all necessary data, another object (which will get the necessary data) can be passed as a method parameter.
In our example, we extracted another class called Address which has all the data related to the user’s address. Thus, we replace three user parameters with a unique object. Moreover, the Address object may be used in classes other than User in future implementations.
Benefits
Makes the code more readable and shorter. Also, may reveal previously unnoticed duplicate code.
🌀 5 - Primitive Obsession
Primitive Obsession is a code smell that can be found when our code use primitives instead of small objects for simple tasks, such as currency, special strings for phone numbers, ranges…
In other words, primitive obsession is when you have a bad practice of using primitive types to represent an object in a domain.
The real problem starts growing when these primitives grow in number along with their behavioral characteristics. It gets worse when developers define the same primitives at different places with code duplication.
That code duplication can result in the creation of a utility class, which is an anti-pattern and doesn’t make sense in an OOP context.
In the example below, we have a class called Site with name and URL properties. But, before defining the URL property value, we have to check if the passed value is valid. Moreover, we may have other methods to handle the URL data, such as the getUrlParts method.
If we look at the Site class, we may come to the conclusion that probably that class shouldn’t be responsible for the URL validation and manipulation.
Furthermore, if we’d had to implement a new class that has the URL property too, we’d need to duplicate all the validators and handlers from the Site class to the new class.
Solution
If you have a large variety of primitive fields, it may be possible to logically group some of them into their own class. Even better, move the behavior associated with this data into the class too.
Coming back to our example, we already understood that primitive obsession may result in code duplication. In this case, the URL validation and manipulation would result in that duplication.
Thus, we can interpret that URL by itself is a type. It has its self validations and handlers. So, we may extract it to a new class URL with its own methods and properties. This way, we can use the URL class as a property in other classes without code duplication, making the code more flexible.
Benefits
Refactoring this smell makes code more flexible, thanks to using objects instead of primitives. Furthermore, it brings comprehensibility and organization to the code.
✅ Conclusion
Throughout our careers as developers, we often need to write code without planning. The consequence of this fact is that we end up creating code smells.
A code smell is any characteristic in code that possibly indicates a deeper problem.
In this article, I showed the 5 most common code smells, in my opinion, and how to refactor those smells and the benefits of doing it. I hope you guys enjoyed the article, if you did, leave claps and comments. If you didn’t like it, let me know why, constructive criticism is always welcome!
📚 References
“Refactoring the Large Class code smell” from Makolyte
