Understanding Code Smells and How to Avoid Them
This blog was originally published in the Typo blog.
Code Smells — A common encounter by developers and testers.
They are tangible and observable evidence that something is wrong with the application’s underlying code. When left unaddressed, it can degrade the application’s performance and increase the technical debt. This further makes it difficult for the software teams to provide value over time and deliver the product faster to the market.
What is Code Smell?
Code Smell was first introduced by Kent Back in the 1990s and popularized by Martin Fowler’s Refactoring Book.
In simple words, code smell is a warning that the source code is messy and isn’t meeting the best practice standards.
However, Code Smell isn’t synonymous with bugs or errors. And they do not always mean that the code is wrong or broken. It highlights the presence of bottlenecks in the codebase that need immediate attention. If not, they can reduce the code quality, readability, and maintainability.
Moreover, Smelly code can easily become rotten code, when not taken care of in early stages. One of the main causes of code rot is technical debt. Hence, it is advisable to periodically check and fix them to prevent both code rot and technical debt.
Top code smells and how to avoid them?
Duplicate Code
Duplicated code is the most common code smell. It happens when a similar code exists in more than one area; most probably because the code was copied and pasted in different parts of the programme.
Although, it may look harmless but becomes challenging since the developer has to make multiple tweaks during feature updates. This not only decreases the code maintainability but also results in inconsistent application as the change wasn’t applied uniformly. It further increases the cycle time and is considered to be a business risk as well.
Solutions
- When there is the same method, create the same Local Variable and reuse it, and during the same class, create common Method refactoring.
- Leverage the power of functions or loops to make code appear once in a program.
- Use refactoring techniques such as the Extract method, pull-up method, and substitute algorithm.
Long method
The long method is when the method contains too many lines of code. There isn’t any specific number of lines that are considered to be long. Some believe it to be 25 while other thinks 50 is too long.
This code smell also violates the single responsibility principle.
Long Method makes adding new features or updating existing ones challenging. It becomes harder to test, understand, and debug the code. This not only increases the cyclomatic complexity but also leads to unexpected bugs.
Solutions
- Establish maximum line counts for methods with your development team.
- Use the ‘Extract method’ i.e. breaking it up into several smaller methods where each of them is doing one precise thing.
- Remove local variables and parameters before extracting a method.
Dead code
The code becomes dead when the developers forget to clean up the existing code, aren’t aware of the dead code in the first place, or can be in the form of old, commented-out code. The code is no longer needed yet it is still present in the application. It can be a variable, parameter, field, method, or class.
The amount of dead code in the application signifies:
- How well projects were managed
- How much the team cares about technical debt
- The amount of communication between them
This makes code hard to understand, increases bugs and errors, and more security vulnerabilities.
Solutions
- Remove the dead code completely after writing the code that replaces its functionality.
- Use static analysis tools or IDEs such as Visual Studio as it suggests to remove the unused code.
- Refactor code to get rid of redundancies and maintain structure.
Lazy class
This code smell arises when a class exists yet doesn’t contribute seriously to the function or behavior of the software.
This increases the code complexity and clutters the code base. Hence, increasing the cognitive load for the developers which costs both time and money.
If left unaddressed for a long time, it can result in future risk i.e. Adding more functionality to the lazy class can lead to a bloated or poorly designed class.
Solutions
- Implement constant code reviews to identify and address lazy class.
- Determine whether the ‘Lazy class’ serves a legitimate purpose in the codebase. If not, then remove it through the remove class technique.
- Use the ‘Inline class technique’ to detect a lazy class and merge it with another class that uses it.
Middle man
Middle man occurs when the class delegates work to another class and it doesn’t have any independent functionality. A few of the reasons behind this code smell are:
- The previous refactoring may have moved functionality elsewhere and left the class empty.
- The middle man was relevant at one point however, it is no longer needed.
This increases the code complexity and creates noise in the codebase. Further, it makes it harder to maintain the code and less efficient without adding significant value.
Solutions
- Document the reasons for removing the middle man to guide developers while code cleanup.
- Use the ‘Move method/ when the method logically belongs to another class which improves cohesion.
- Use the ‘Inline function’ when only a few class methods are not delegating and need to inline them into the caller.
Primitive obsession
Primitive obsession is a type of code smell that developers can’t identify intuitively.
It occurs when a primitive value controls the logic in a class and represents complex concepts or behaviors. In simple words, when a code relies too much on primitive values.
Using primitives for everything is certainly a bad practice. This leads to poor readability, validation, and abstraction.
Solutions
- Replace the data value with the object if the primitive fields logically belong together.
- ‘Introduce a parameter object’ to represent the data and clean up the code base.
- ‘Preserve the whole object’ when its state is needed together. Avoid extracting small parts of objects to pass around.
God objects
God objects are one of the common and problematic code smells. It is when a single class or program is central to the system i.e. handling diverse tasks that are not cohesively related.
It violates the single responsibility principle and creates tight coupling and challenges in code maintenance.
God objects use more unwanted resources even for simple operations and make it difficult to isolate and test components effectively.
Solutions
- Refactor the class into smaller, more manageable classes. Each should hold a single responsibility.
- Apply design patterns such as Facade, mediator, or delegation to create clearer interaction between classes.
- Structure code into independent, reusable modules.
Feature envy
This code smell arises when a class accesses the data or method of another class more than its own. It is because the class’ functionality is too closely tied to another class.
Feature envy violates the ‘Law of Demeter’ — The objects should only talk to their immediate friends, and not access their internal data and methods of other objects.
It can indicate a poor design that doesn’t include the encapsulation and cohesion of objects. It also results in high coupling between classes.
Solutions
- Look at the code and identify the class reference. Use the ‘Move method’ to consider moving relevant methods to those classes.
- Use the ‘Extract method’ to move the part in question if only part of a method accesses the data of another object.
- Apply design patterns such as strategy and visitor.
Large class
In this, the class contains many fields/methods/lines of code/responsibilities. This violates both the single responsibility principle and the open-closed principle.
It indicates a weakness in the design and makes it difficult for developers to understand, read, and maintain the code. Moreover, it increases the chances of errors and harder to locate them.
Note that, God objects often manifest as large classes. However, not all large classes are god objects.
Solutions
- Keep the classes small and adhere to the single responsibility principle.
- Use the ‘Move method’ to move a method or field to another class that is more closely related to it.
- Ensure thorough testing before and after code refactoring to maintain the codebase.
Improper names
The improper names of variables, classes, and functions indicate that the code is not clean. This could happen when it includes overly abbreviated names, non-descriptive names, or using different name schemes.
This leads to an increase in the cognitive load of developers and makes them suffer from ambiguity. It also lacks precision and leads to more confusion and errors.
Besides this, improper names make pair programming and knowledge sharing challenging for the developers.
Solutions
- Keep the variable names short and descriptive and the function class should include one verb describing what they do; without adding too many words.
- Adopt consistent naming conventions among the development team.
- Use code analysis tools to detect naming style violations and suggest improvements.
Comments
Unfortunately, comments are code smells too. While it is a good practice, when it is overused for every step, it creates excessive noise in code. This decreases the readability and maintainability.
The comments can be inaccurate too since the reviewer shares them based on their perspective and understanding.
The comments should only explain the ‘Why’ and ‘What it is doing’ part of the code. And not explain ‘How’ the code works. If this is the reason, it could be that the code is not self-explanatory and needs refactoring. Besides this, long, dense blocks of text can disrupt the visual flow.
Solutions
- Use the extract function to explain what a block of code does.
- Remove comments; rather, rely on clear and descriptive functions and variable names to convey the code’s purpose.
- Explore pattern techniques or libraries that can enhance code clarity without relying on comments.
Long parameter list
A long parameter list occurs when there is a long list of parameters in a method or class. Usually, the maximum number of parameters in a method should be 3 or 4. Otherwise, it tries to handle too many responsibilities.
It decreases the readability and reusability and makes it prone to errors and bugs. It further makes it harder to test and difficult to debug.
Besides this, It can become challenging to reuse the method in different contexts since it might require specific combinations of parameters.
Solutions
- By the ‘Introduce a parameter object’ method, create a new object from the list of parameters and transfer it as a single argument.
- Use ‘Preserve whole object’ when the parameters belong to a single object.
- Use ‘Replace parameter with method call’ when some of the arguments are just results of method calls of another object. This object can be placed in the field of its class or passed as a method parameter.
Shotgun surgery
Shotgun surgery happens when developers have to make lots of small changes to the codebase. The code smell often overlaps with other code smells, especially duplicate code.
It might be scattered around a much larger class or may even be in multiple classes or different parts of the codebase.
This type of code smell forces a clumsy, error-free approach and unnecessarily adds complexity to the codebase. The changes consume more time and increase the cognitive load of the developers since they have to remember the changes in various places.
Solutions
- Document clearly how many files are used while making conceptually simple changes.
- Refactor and adhere to the single responsibility principle by handling multiple concerns into smaller, focused components.
- Reduce the tight coupling between classes either by applying the ‘Dependency injection’ technique or using design patterns like observer or strategy patterns.
Inappropriate intimacy
Inappropriate intimacy occurs when a method has too much intimate knowledge of another class or method’s inner workings or data. It means bi-directional behavior between classes that are tightly linked to each other.
Changes in one module can easily break the other due to their deeply intertwined nature. This results in difficulty in enhancing/extending features and bug fixes.
Inappropriate intimacy also reduces modularity, flexibility, and testability.
Solutions
- Use the ‘Encapsulate field’ when inner data needs to be exposed instead of being private.
- Use the ‘Extract interface technique’ to define a common interface for the classes that need to interact with each other.
- When two classes are too related yet don’t talk much to each other, they need a split, merge, or refactor.