Beware the Small Smells
There are varying degrees of code smells. Some can be complete showstoppers for development/maintenance process while others are merely minor nuisances that might slow you down a bit. These kinds of smells can range from violating agreed-upon naming schemes to using global variables to exposing a member variable more than necessary. By themselves, these code smells probably won’t cause you to break down in tears when it’s time to refactor or add a new feature to your code. After all, there are more important things to do: new features to add, larger smells to tackle and the like. Someone will get around to fixing them, right?
Letting the small smells persist in your code base leads to larger smells moving into the neighborhood. Small smells can become large smells, leaving the door open for more small smells to fester. The idea is akin to that of a house with a couple of broken windows - so what if I break a third window? It’s not like the house is in worse shape because of it. It already has other broken windows! If a bunch of small smells gather around areas of your code, they’ll grow into full-fledged bugs and other nasty smells that will cause maintenance nightmares. Small code smells need to be nipped in the bud and taken care of as quickly as possible. The longer they remain, the longer your code rots.
Not all small smells are left unattended by choice; for example, you have deadlines. Unfortunately, developers do not have an infinite amount of time to work on code. Features need to be developed in a reasonable amount of time, or else the clientele would not remain the clientele for much longer. Thus, you wait to fix smells until later. You keep track of the places you want to go back to. You’ll have time to come back and finish what you started later, right? Again, while ideal, this is not going to happen. I think I’m repeating myself here, so I’ll move on.
The bottom line is that you have to spend some time clearing out the code smells. The question is this: how can you achieve the maximum benefit from refactoring while spending the least amount of time looking for where to clean up your code? One option is to keep track of all the smells you came across but did not have time to fix. There are several ways you can achieve this: place a TODO comment by every smell in your code; keep a separate listing available in a text document; create tasks in your project management software, and so on. TODOs can be easy to search for and take you right to where you need to go, but this litters your code with comments. As we all know: comments are code failures. If you have a separate list, it will invariably become outdated: smells that are fixed won’t be removed and newly-discovered smells won’t be added. Creating stories, issues, bugs, etc. in your project management system will make sure you don’t forget smells. This does, however, suffer the same fate as maintaining a separate list because you will also have to add a new story every time a smell is discovered.
Basically, maintaining a list of smells to fix as you’re developing is not the best way to go. Fortunately, you’re not out of luck just because you don’t have time to fix the small smells as you are developing. Nitriq can lead you straight to all of your smells, big and small. The predefined queries sniff out the code smells and tell you exactly where to find them. The rest of this post will focus on the small smells in this example usage of one of our team’s newest products. We’ll take a look at some code smells, use Nitriq to locate them, fix the smells and re-analyze the code to verify that the smells have been eliminated.
Let’s first examine a simple rule: declaring protected members in sealed types. In C#, when a class is sealed it cannot be inherited. Protected members by definition have private accessibility but can also be accessed by any class that derives from the parent. If the parent class is sealed, there’s no reason not to make the member private. These members could have been declared in classes that were unsealed at one point, or classes that were about to become unsealed but never were. In either case, the accessibility of the members should change so that when future developers work on the project they will not have any ambiguity as to what the accessibility of the class or its member variables should be.
Here’s a code snippet I inserted into my Project Euler solution:
public sealed class IHaveNoSon
{
protected int _age;
}
Now let’s run the relevant query in Nitriq:
Success! It has found the class IHaveNoSon and has determined that it has a single protected member. Now let’s go in and change the accessibility of _age:
public sealed class IHaveNoSon
{
private int _age;
}
And now we re-analyze the assemblies and run the query again:
As is evidenced in the screenshot, we have eliminated all instances of this rule being broken from our code base. Excellent!
A second rule available out of the box in Nitriq is “properties should not be write only”. I don’t really have to elaborate on why this is a code smell: what’s the point of storing a value that you don’t have access to? Running the query to catch this smell functions just like the above example, only the query “Properties should not be write only” would be run instead of “Do not declare protected members in sealed types”. Another nice rule that needs no explanation is making sure that all created exceptions are public. If you have created an exception it cannot be thrown if it is not accessible! What’s worse is that there will be code that will never be used, creating rot in your code base. The last thing the aforementioned house needs is a door that leads to nowhere. In Nitriq, the “Exceptions should be public” query will easily find all the non-public exceptions in your code.
Letting small smells continue to survive is not a good practice. Not only can small smells become big smells, but it also lets other smells pile up in your code base. While it is not always possible to fix every small smell you come across due to time constraints, it is not practical to maintain a list of smells to be fixed some time in the future. It is a lot easier to let a tool do the work for you. Nitriq can automatically find the common smells (and some not-so-common smells) in your code base quickly and point you directly to the offending class, method, field, etc. Using this tool can speed up refactoring while also easing your conscience for leaving the smelly code around to begin with.
This post is part of a series exemplifying how Nitriq can assist in the identification of code smells.


