Code smells are a set of common signs which indicate that your code is not good enough and it needs refactoring to finally have a clean code. In this article, I am going to explain the code smells with real-life examples from real projects on GitHub and show you the best way to refactor these smells and clean things up. I also suggest you read this article afterward. “Advanced Coding Skills, Techniques, and Ideas”
– Duplicated Code and Logic Code Smell
It is common that developers are kind of lazy, which is very good in so many ways; however, being lazy and copy/past lines of code is not a proper behaviour. It could lead to the most common code smell which is the logic duplication as in the following example.
So, to get rid of this code smell we need to extract the red part into a separate method which helps us to reuse it wherever we need.
– Long Methods and Classes Code Smell
Well, I believe we all made that mistake when we needed to add another if() or for() statements to an existing method in order to validate user inputs or to check if the user is logged in. So long story short, you shouldn’t do that. If you need such validation, then create its own method. The perfect method should be between 4 to 20 lines. If it is more than 20 line, you probably can extract a couple of lines into a new method. Same rules here for classes too, smaller is better especially if you apply the Single Responsibility Principle.
– Duplicated Methods in the Same or Different Class Code Smell
Another code smell that needs to be taken into consideration is when you have two methods that do the same functionality. The following image should make it clear.
– Class Divergent Change Code Smell
If you ever read about the SOLID principles before, especially the Single Responsibility, then you know that one class should have only one reason to change. This means a User class shouldn’t have a function related to products or file conversion. You can easily fix this code smell by extracting the unrelated method to a new class like Product class or FileSystem class.
– Shotgun Surgery Code Smell
It is the exact opposite of divergent change. This code smell will make you change many classes for one single reason which means that many classes have the same reason to change in order to apply one behaviour. For example, you need to create a new user rule such as ‘Supper-Admin’ then you found yourself must edit some methods in Profile, Products and Employees classes. In that case, consider grouping these methods in one single class so this new class will have a single reason to change.
– Feature Envy Code Smell
Sometimes you found a method in your class that extensively makes use of another class. In that case, you may consider moving this method to the other class it uses. Have a look at the next image. Wouldn’t it be better if the getFullAddress() becomes a part of ContactInfo class instead of User class since all it does is to use ContactInfo methods.
– Data Clumps Code Smell
Sometimes you find so many functions that almost take the same parameters list. This kind of parameters, that is always used together, causes the data clumps code smell. Take a look at the next example; you will find that almost all kinds of reservation require the passport information.
In that case, it would be much better to move the passport information into its own class then pass the PassportInfo object to the reservation methods. This is an excellent example of code reuse and remember that long parameters list can lead to code failure, conflict and difficult unit testing.
– Primitive Obsession Code Smell
This coding smell appears when you start using primitive data-Types everywhere in your application. For example, using the integer for phone numbers and string for currency sign. If that is the case with you, take a look at the following class.
As you can see, the address here is defined as an array. This approach mainly will cause two problems, such as every time we need the address we will have to hard code it. So, why not we create a new class called Address.
Now, every time we need to add/edit an address we hit the Address class. Also, any time we need to add a new “contact us” method we hit ContactUs class. So, each class has a single responsibility.
– Switch Statement Code Smell
Maybe you wonder why the switch statement is terrible. Well, it is not always bad, but if you can notice in the next example, the switch statement is big and unextractable by its nature. So when it becomes huge, you can’t divide it into smaller methods.
Just remember! if your switch statement is not big, then you can leave it. An excellent example of switch statement is within the Factory design pattern.
– Parallel Inheritance Hierarchies Code Smell
Sometimes I wonder whether the Parallel Inheritance Hierarchies is really a bad practice. Well, let’s first explain what Parallel Inheritance Hierarchies is. Then decide if it is a bad thing or not.
So, as you noticed from the image above that every time we create a new department class we also need to create a privilege class which leads us to the “Shotgun Surgery” code smell.
– Lazy Class Code Smell
A Lazy Class is the one that doesn’t do so much. Do you remember this image from above?
We decided to move the address to a separate class, but we didn’t do the same with the hot-line because it would be a class with 3 lines only. So, whenever you found these lazy classes, you should eliminate them.
– Temporary Fields Code Smell
Temporary Fields code smell happens when you have a class instance variables that have been used only sometimes. Have a look in the next example; you will notice that $name and $contactDetails are only used in the notify() method.
So why not passing them as a method parameters.
– Message Chains Code Smell
Message chains is the code smell when you have a class that uses another class which uses another class and so on. In the following image, you can see the following chain, Employee->EmployeeConfig->Config
So you can make your code cleaner by shortening the chain to,
– Inappropriate Intimacy Code Smell
Sometimes you find a method in a class that needs to know too much about the inner workings or internal data of another class. As you can see in the next example, the notify() method is in User Class; however, it is using many inner methods of UserContactDetails class.
In that case, it would be better to move this logic from User class to UserContactDetails class and create getWelcomeMessage($userName).
– Middle Man Code Smell
Sometimes you find many methods in one class do nothing but delegating to another method in a different class. In that case, the first class is considered as a middle-man class, and most of the time it would be better to get rid of it.
Note: Middle Man classes could be helpful in some cases as in the Facade design pattern.
– Alternative Classes with Different Interfaces Code Smell
Usually, it happens because of the lack of communication between the team as two different classes are created which do the same thing which means code duplication.
– Incomplete Library Class Code Smell
Third-party libraries do not always provide you with all the functionalities you need in your application. In the next example, a library that handles documents can retrieve one document by its ID or retrieve all the documents at once.
So, what happens if you need to retrieve all documents of a particular user? Remember that it is horrible if you tried to edit third-party classes on your own. In this case, you need to extend the functionality of the Document class without editing the original class. Well, the decorator design pattern can be helpful here as you can see in the next image.
Now, you should start using DocumentsDecorator class instead of Documents class.
– Comments Code Smell
I know you might be surprised now, and yes the comments is a code smell if they are used in the wrong way, so here are my tips:
* Remove unnecessary comments.
* If the code is obvious, don’t write a comment.
* Don’t leave commented old code.
* Remove commented debugging var_dump, echo, ..etc.
– Speculative Generality Code Smell
This code smell is about premature optimization, many developers fail to see this one. Some notes to be considered during planning are:
* Don’t over plan your code.
* Don’t try to cover a case that likely has 1% chance to happen in the future.
* Sacrify some speed in order to make your algorism simpler, especially if you don’t need a real-time result from your application.
* Optimize for speed when your application is actually slow not when you only have 100 users.
Now we have covered the code smells and the best ways to clean them up, so you are more than ready to write very clean code and refactor your old classes. Wish you all the best!
More to Read: