Sharpen your sense of (code) smell
As humans, we have plenty of sweat glands all over the skin. As coders, we have plenty of lines of code all over the project repository. In humans, some of those glands contribute the body odor – good or bad. In coders, some of those lines contribute to code smell – good or bad. In software, a code smell is not pleasant.
Aside from obvious hygiene and social considerations, in much the same way a strong and unpleasant body smell may be the surface indicator of a deeper medical problem, a strong and unpleasant code smell may be the symptom of relevant weaknesses in the code design. Should we be worried about code smell?
A code smell is not the same as a bug. More simply, a code smell is a piece of code that we perceive as not right, but don’t fix right away. This is the crucial point. And it is often a sore point.
The growth of a code base can be compared to the growth of a tree. Pruning is crucial to keep the tree healthy and increase production of flower and fruit. Without pruning, branches get longer and longer and mostly produce fruit at the tips. Without refactoring, code smells may ultimately increase technical debt.
Code smells primarily affect the maintainability of a software system, and any code is almost immediately in need of maintenance as soon as it’s written. Code smells have fancy names and apply to different coding scenarios.
This article is the first of a series in which we’ll discuss various code smells and a few aspects to refactor, to help improve our code base now. Because – let’s face it – you’ll never have the time to clean it later.
In this series:
- Sharpen your sense of (code) smell
- The simple case of special string types
- Join data items that want to go together
- Easy conversions and readability
- Put it down in layman’s terms
- Every method begins with “new”
- Dependency injection doesn’t strictly require frameworks
- Super SuperClasses
- Null pointers: an opportunity, not an exception
- You ain’t gonna use it!
Let’s start at the beginning and discuss the various types of code smells.
Method-level code smells
The first thing you should check in a method is its name. Equally important are the parameter list and the overall length. Typically, the ideal method:
- Is clearly and appropriately named
- Is no longer than 30 lines and doesn’t take more than 5 parameters
- Uses the simplest possible way to do its job and contains no dead code
Here’s a list of code smells to watch out for in methods, in order of priority.
|1||Dead code||The method is not used.|
|2||Lazy object||The method does too little work.|
|3||Middle man||The only work the method does is delegating work.|
|4||God method||The method does too many things.|
|5||Long parameters list||The method takes too many parameters.|
|6||Contrived complexity||The method has an unnecessarily complex implementation.|
|7||Cyclomatic complexity||The method has too many branches or loops.|
|8||Inappropriate intimacy||The method depends too much on the implementation details of another method or another class.|
|9||Feature envy||The method accesses the data of another object more than its own data.|
|10||Black sheep||The method is noticeably different from all other methods in the same class.|
Class-level code smells
The first thing you should check in a class is if its name and programming interface reflects its purpose. Typically, the ideal class nicely models an entity in the business domain and does that using an appropriate business language.
Here’s a list of code smells to watch out for in classes, in order of priority.
|1||Lazy object||The class does too little work.|
|2||Middle man||The only work the class does is delegating work.|
|3||God object||The class does too many things.|
|4||Primitive obsession||Too simple, primitive types are used to model data with some special meaning.|
|5||Tell don’t ask||The programming interface of the class is not sufficiently focused on performing specific operations.|
|6||Indecent exposure||The class unnecessarily exposes its internal details.|
|7||Inappropriate intimacy||The class depends too much on the implementation details of another class.|
|8||Refused bequest||The class inherits from a base class but only some of the inherited behavior is really needed.|
|9||Speculative generality||Design of the class is overly complex due to hooks for features that will possibly be introduced one day.|
|10||Temporary field||The class has a member that is not significant for the entire lifetime of the object.|
General code smells
Higher than the abstraction level of classes and methods, there are a few other aspects of code you need to focus on. The most relevant is making sure that every of piece code clearly communicates its intent. This relates to the naming convention, the (spoken) language in which naming is expressed, and imperative approach.
Here’s a higher-level list of code smells to watch for, in order of priority.
|1||Lost intent||The code doesn’t easily communicate its purpose.|
|2||Oddball solution||The same problem is solved in different ways.|
|3||Combinatorial explosion||Multiple pieces of code do the same thing but using different combinations of parameters.|
|4||Duplicated code||Nearly identical code exists in more than one class or method or library or system.|
|5||Contrived complexity||The code has an unnecessarily complex implementation.|
|6||Solution sprawl||The implementation of a responsibility sprawls across multiple classes or methods or libraries.|
|7||Divergent change||You have to change many unrelated methods when making one change to a class or library.|
|8||Shotgun surgery||Making any modifications requires that you make many small changes to many different classes.|
|9||Deodorant comments||Nice words are used to perfume bad code.|
|10||Data clump||Group of variables always passed around together.|
Some extra thoughts should be spent about comments in code. While describing the intent of the method at the top of its implementation is anyway useful for whoever happens to read it, commenting implementation steps might be arguable. The risk of “deodorant comments” is that you use comments to smooth the natural (bad) odor of the code.
The perfect code is intention-revealing and as such, it needs very little comments. Comments are insightful when they document the wherefores of a technical decision, points left deliberately open, doubts, or future developments.
As emphatic as it may sound, comments should never state the obvious. The intention-revealing nature of code is also increased by automated tests, especially those done not simply to increase the percentage of calculated code coverage.
Code smell stereotypes
There are some stereotypes about code smells as well. One states that code smells are introduced during the evolution of building software. A code smell very often is simply a bad habit or due to particular circumstances. There’s no reason for not committing well-written code right the first time. It’s about how every single developer writes their code and anyone should aim at writing it right, immediately. By default!
Quality at commit time can be enforced to some extent, by using check-in policies. Beyond their actual effectiveness, check-in policies can help prevent committing less-than-optimal code.
Another stereotype is that refactoring removes code smells. Well, refactoring is about writing code and as such, it is an activity that can be done in a good or bad way. Solution Sprawl, Contrived Complexity, and even Oddball Solutions can be easily added with the best intentions during refactoring especially if the vision of the entire project is limited.
Finally, all developers can introduce code smells. Worse yet, under pressure, especially seasoned developers with the highest workloads, are more subject to shortcuts that may result in code smells. Code inspections, if planned, should occur right after tight releases to be quickly effective.
The bottom line
Most of the smell we perceive is about the logical distance between the abstraction level of the programming language and the language of the business. The closer we can move the expressiveness of the programming language to the business, the more readable and granular our code becomes.
Granularity, modularity, separation of concerns, and all the wonderful theoretical concepts we may have heard above become concrete and factual when we get guided by the idea of making our code speak the language of the business.
So much for code smells. In our next post, let’s look at a practical example: special strings! See you next week!
Download ReSharper 2018.1.2 or Rider 2018.1.2 and give them a try. They can help spot and fix common code smells! Check out our code analysis series for more tips and tricks on automatic code inspection with ReSharper and Rider.
Subscribe to Blog updates
Thanks, we've got you!
Eager, Lazy and Explicit Loading with Entity Framework Core
Entity Framework Core (EF Core) supports a number of ways to load related data. There’s eager loading, lazy loading, and explicit loading. Each of these approaches have their own advantages and drawbacks. In this post, let’s have a quick look at each of these ways to load data for navigational prope…
OSS Power-Ups: bUnit – Webinar Recording
The recording of our webinar, OSS Power-Ups: bUnit, with Egil Hansen and Steven Giesel, is available. This was the twelfth episode of our OSS Power-Ups series, where we put a spotlight on open-source .NET projects. Subscribe to our community newsletter to receive notifications about future webi…
Accelerating Your Testing Workflow with Unit Test Creation and Navigation
Unit tests play an important role in our daily development workflow. They help us ensure our codebase's correctness when writing new functionality or performing refactorings to improve readability and maintainability. In the process, we often create new test files that accompany the p…
Introducing Predictive Debugging: A Game-Changing Look into the Future
With the introduction of debugging tools, software developers were empowered to interactively investigate the control flow of software programs to find bugs in live environments. At JetBrains, we've always strived to improve the art of debugging. Besides the more standard things you expect from a de…