Code smells to avoid for a healthy codebase
We often write code to the best of our knowledge, but slowly over the weeks, we get a feeling that something is not right. We sense some discomfort with the code but still do not understand clearly what the problem is.
This intuition is correct and relatable. These are bad practices, also known as code smells.
As a developer, we should be smart enough to sense this code smells even before writing our code. This not only leads to cleaner solutions but also increases the productivity of our team and simplifies code maintenance over the long run.
Here are some common code smells that once should pay attention to while coding.
Duplicated code
Imagine a situation where we want to change functionality. We realise that the code that needs to be modified is lying in multiple places and we need to visit each section and update the same code to avoid any bugs. This gets frustrating for any developer and impacts the ease of code maintenance.
This common anti-pattern present in very professional codebases too, and one should try to eliminate it as much as possible to keep the code clean and avoid changes in the same code in different places if a feature changes.
There are different ways to help remove duplicated code, but the most common solution and the easiest is to create a new function to encapsulate that logic.
In case the code is somewhat similar and is not exactly the same, one should be able to rearrange the code so the similar items are all together for easy extraction.
Remember the DRY principle (Don’t repeat yourself).
Long Functions
To write all the code in just a single method sounds easy and quick, but this habit can result in bad user readability and debugging.
Writing code for humans is the golden rule of being a good programmer. Long functions/methods are one of the worst enemies of this mindset since they make our code less readable.
Functions need to be descriptive about what they do and must follow the Single Responsibility Principle (S from the SOLID Principles). It should focus on a single and isolated task so that we can reuse them when necessary.
The code becomes less maintainable when the number of lines in it increases and thus increasing its complexity.
The quickest way to avoid this is to identify a definable logic and extract that into a new function that can be given a simple name.
Long parameter lists
Have you ever seen a method that takes 6, or 7 or more parameters? Do you believe it creates a lot of confusion about the order and type of arguments?
Well, modern-day IDEs help us with hints to indicate what the method wants exactly and at what point; but this is being exploited by the new developers. Having such a long list is not only prone to bugs but also reduces readability.
We could usually solve this problem by introducing a parameter object. It consists of combining the long list of parameters that fit together into a new object and pass it as a unique argument. In other words, it is also known as removing data clumps. This has many benefits since we can define the parameter object before the function call and the property order is irrelevant.
In newer programming languages, like Kotlin, we can have a default value for the parameters and thus pass only the ones relevant at the time of function invocation.
Too many comments
Good code is self-explanatory. Having better naming conventions, abstractions and indentations can very well display the logic of the code.
It is preferable to have cleaner coding habits than having bad ones and trying to cover them up using comments.
Too many comments increase the time to understand the code and thus reduces the turn around time to add new features or fix issues. It also increases the file size and maintainability of the code base as the developers now have to update both code and comments.
Comments just act as a boilerplate that developers should avoid.
When you feel the need to write a comment, first try to refactor the code so that any comment becomes superfluous.
Martin Fowler.
Mysterious names
Proper naming conventions to variables, functions, and classes can give a strong boost to the cleanliness of our code. It assures more descriptive code, reduces the usage of comments, and make future developers’ life easier while reading the same code!
A machine understands 0 and 1, and our code is translated later to that language. But what we write is for human beings. Write your code as you would explain it to a 6 years old child.
Global data
A variable that is accessible from everywhere and, especially is mutable, can generate unexpected behaviours if modified. The most common example of global data is global variables.
The first defence mechanism we can use is to Encapsulate the Variable within a function. This at least provides a better way to control its access and debug where it has been modified.
Dead code
Sometimes we write new code with the purpose of using it later or to make our code cleaner and more descriptive. But in the end, we generally end up not utilising it.
This unused code becomes useless and adds up to the whole codebase without any reason. This sometimes not only creates confusion at a later stage but also adds to tech debt of the codebase.
We should delete this code to keep our source code clean. At any situation, if we would like to get that code back, a Version Control System like Git can fetch it for us in seconds.
Large classes
This is also a clear indication of a violation of the Single Responsibility Principle.
When a class starts growing and has too many fields, the duplicated code is staring right at the develop. We should take care of keeping our classes as small as possible.
The first step to refactor this is to remove redundancy inside the class itself. We should then decouple the code using class extraction.
We should start by taking long methods with a lot of shared code and split them into smaller methods. This way, we can compose them as we want to and avoid any sort of repetition.
Inconsistent Names
We often tend to divert from the good naming conventions and end up creating a new name for a generic functionality.
A file open and close operation could simply be called as open() and close() respectively. There is no harm in using a generic name for known functionalities. This habit simplifies the development process and also speeds up code debugging.
Shotgun Surgery
If a change in one class requires cascading changes in several related classes, we should consider refactoring so that the changes are limited to a single class.
This anti-pattern creates a burden of work for even a small change in feature or bug fix over the lifetime of the source code.
Feature Envy
A method accesses the data of another object more than its own data. This smell may occur after fields are moved to a data class. If this is the case, we should move the operations on data to this class as well.
As a thumb rule, if things change at the same time, we should keep them in the same place. Usually, data and functions that use this data are changed together (although exceptions are possible).
We may either Move methods or Extract methods depending on the kind of functionality that needs to be moved across.
Wrapping up
It is always good to discuss code and it’s health. Code is written once but read multiple times. These small changes pay off in the journey. There might be some hurdles in the beginning but eventually, it benefits the product and the people working towards it.
