The term “code smell” is probably something you have encountered if you have been developing software for a few years. Even if you haven’t come across the term you’ve probably encounter examples of them.
I’ve been developing software now for 4 years and I’ve started to pick up on a variety of examples of code smell. Some of which I have done in the past, while others were from working with other people’s code. In this post, I’ll be sharing some of these common code smells.
What Is Code Smell
Code smell is not bugs or errors. They only surface once you start digging under the application. For the most part, they are implementation details that end up decreasing the overall quality of the code.
I like to think of code smell as warning signs with the code. The software will work, but the performance might be slow. The code might be more vulnerable to bugs. When everything is as expected then the code does what it needs to do, but when an unexpected case happens the software blows up. These are only some of the results of code smell.
What to Look For
The more you code the more you improve your “code nose”. Code that doesn’t trigger off a warning bell back then starts to. It should also be noted that code smell is not something that should be hard to find just like how a bad smell is not difficult to notice. With that in mind, here is a non-exhaustive list of code smells I have encountered.
Comments
Are there excessive comments? Do the comments explain the “why” or the “what”? If the comments are explaining what the code is doing then it should be refactored so the comments are not needed. Readable code is self-commenting and doesn’t risk becoming stale over time as things changes.
Long Parameter List
Does a method have many parameters? The more parameters a method has the more complex it becomes. If there are too many parameters, it might be a sign that an object can help group some of them together to make things simpler.
Duplicated Code
Duplicated code is the bane of software development. Whenever you find duplicate code you should address it as soon as possible. Refactor it into a single method. Why? Because when you make a change it applies everywhere. Can you imagine how painful and error-prone it would be to go through all the code and make changes to each piece of code doing the same thing?
Long Method
When a method gets long it is usually the case that it is doing way too much. If a method is doing multiple tasks, each of those tasks should be refactored to its own method. Each method having one responsibility makes things simple.
Large Class
Similar to a long method, a large class is difficult to understand and troubleshoot. It is likely the class has way too many responsibilities. If that is the case, a restructuring or breaking things down into smaller classes can help.
Dead Code
Code that is commented out and has no explanation as to why should be deleted. It frees up space and eliminates potential issues in the future. Also, you’re most likely using source control so you can always get it back.
Not Meaningful Name
Does the name of a variable or method convey its purpose? Can you read the name and know what it does? If you need to dig into the implementation details then rename it.
Generic Solutions for Something Specific
You should write code to solve today’s problems and worry about future problems when it actually is necessary. It can get easy to become lost in the “what if…” design, which greatly increases the complexity of something. Remember YAGNI (you aren’t gonna need it).
Data Only Class
A class should contain data and methods to operate on that data. If a class only has data, then the operations on those data might be scattered across the code. Those operations should be brought into the class so the operations are centralized. This is a variant to duplicated code.
Inappropriate Encapsulation
Be on the lookout for classes that interface with each other in inappropriate ways. For example, a class communicating with another class as if it knows the implementation details. Classes should know as little as possible about each other. The way classes communicate should be through exposed methods.
Lazy Class
A class should serve a meaningful purpose. If a class is not doing enough to pay for itself, then it can be collapsed or combine into another class. Each additional class adds complexity to a project so it’s best to keep the number of classes as lean as possible.
Middle Man
These are classes that delegate all their work. It might be a class that is a wrapper over other classes or existing functionalities in a framework. Cut out the middle man and go directly to the source.
Cascading Changes
If a change to one class requires changes to several related classes then consider refactoring. Ideally, changes should be limited to a single class. When changes cascade to other classes there is likely a problem such as tight coupling.
To wrap up, your ability to detect code smell is an ongoing process. The more experience you have the more code smells you will find. Some code smells in a project are unavoidable, but in due time you’ll fix more code smells than you will contribute.
I hope this post was helpful to you. If you found this post helpful, share it with others so they can benefit too.
If you want to learn more about code smells I have two posts you can check out. One of them is about anti-patterns that contribute to code smell. The other is about habits that contribute to code smell.
To get in touch, follow me on Twitter, leave a comment, or send me an email at steven@brightdevelopers.com.