Code Smell: Something stinks. Is it my code?

In this article, you will learn how to detect and avoid common bad software design practices, known as “code smells”.

Code smells | Vestiaire Collective

For the past few years as a Software Engineer, I thought my fingers and StackOverflow were the only tools needed to excel at this job; little did I know that my sense of smell 👃 would be useful too. Before you start doubting your abilities as an engineer, know that this is all in a figurative and derivative sense. You can quickly pick out a smell without deep thought or process. The same applies to engineering smells. Do you remember when you looked at a method and something in your heart just gave off signals (off as in off like off ok!)? If yes, then your programming nose is as functional as the monolith. If not, then this article is for you.

Code, like any entity in life, often snitches on itself. As the good detective, you are, it is time to see, or at least smell, the signs when they avail themselves. By definition:

A code smell is any characteristic in the source code of a program that may indicate a deeper problem.

The concept behind this goes beyond the mere amalgamation of functional failures but rests on the premise that it doesn’t have to fail to look weird.

In the spirit of kaizen, it is imperative to continually improve, as such code refactoring will always be with us. With that in mind, I believe that after reading this, you will have an idea of what to improve in your codebase.

Let’s look at the different types of stink. By the way, “it doesn’t have a nose, but it can stink”. We have 5 major smell groups as described in the book Refactoring by Martin Fowler and Kent Beck, namely Bloaters, OOP Abusers, Change Preventers, Dispensables, and Couplers.

At a high level, it seems that anything that violates SOLID principles can potentially be considered a code smell. But as you will see, it extends beyond the SOLID presupposition.

Bloaters

These are the most obvious, not because they look any different, but because almost everyone I know has had a bloated stomach. “Bien sûr” — pardon my french, it’s not a pleasant experience. Unless it’s an extremely rare situation, it is not customary to have a method with more than 1000 lines of code or a signature of the same with more than 7 parameters. A bookish definition of bloaters will be as follows.

Bloaters are code, methods, and classes that have increased to such mad proportions which render them hard to maintain.

Examples include long methods, long parameter lists, primitive obsessions, and data clumps.

Long parameter lists are often a consequence of primitive obsessions. Smart programmers are often obsessed with the use of the heap, and as such, they will prefer to use primitive types as much as possible. It’s a noble quest but anything done in excess can be an issue.

Note: All the code snippets in this article are in Kotlin, but the concepts can be applied to any language of your choice.

fun sendMessage(
  id: Int,
  message: String,
  includeAttachment: Boolean,
  isBlocked: Boolean,
  characterLength: Int,
) {
    /* Some code here */
}

Could be:

fun sendMessage(
  message: ChatMessage
): {
      /* some code here */
}

data class ChatMessage(
  val message: String,
  val includeAttachment: Boolean
  …
  …
)

Where the arguments are just a data class with the properties above. Overloading the heap shouldn’t worry you much, garbage collectors will not miss unless there is a leak somewhere.

OOP/ OOD Abusers

Decision Fatigue

Where do I start here? I know we all love Object Oriented Design but sometimes we overuse it. Remember the time you created an interface that was only implemented by only one class? Why did you do that? If it was for testing purposes, I might understand. Object Oriented Abuse stems from the incorrect usage of object-oriented design.

Here are some examples:

1. Alternative classes with different interfaces — When two or more different classes appear to perform similar tasks but have different signatures. A common culprit is util classes.
2. Switch statements — Complex switch, when, or if-else clauses.
3. Refused bequest — a clear case of inheritance abuse. Imagine a subclass that overrides all superclass methods.
4. Temporary Fields — This often happens when you create a field that is used by only one method in a class, so the field will lie idle for most of the time during the class’s existence.

Change Preventers

Refactoring a class | Refactoring Guru

These are probably the most annoying of all. Imagine one change cascading to a million other places. This can happen if you use magic numbers instead of constants. Change one magic number and you have to update your code wherever the same exists.

These are the main Change Preventers:

1. Divergent Change — Many changes made to a single class (kind of violates the Single Responsibility Principle)
2. Shotgun Surgery — Single change that cuts across multiple classes simultaneously.
3. Parallel Inheritance Hierarchies — When one inheritance tree depends on another inheritance tree.

Dispensables

An example of dispensable comments | Vestiaire Collective

If you don’t need it, don’t include it. So these don’t affect code readability or functionality, but their presence may be unpleasant. Examples are

1. Comments — Yes, comments can be smelly sometimes, the best comment is the name of your class or method.
2. Data classes — We don’t always need them, sometimes it’s enough to use Pairs or Triple. If you don’t need one, don’t create one.
3. Duplicate code.
4. Dead code.
5. Lazy Class — If a class doesn’t capture your attention anymore, it’s probably no longer needed.
6. Speculative Generality — Yes, that unused class or method. Often a result of “just in case” code.

Couplers

An example of Feature Envy

These bad coding patterns are often the result of excessive coupling.

1. Feature Envy — This occurs when a class method often accesses the data of another class more often than it does its members.
2. Inappropriate Intimacy — I like to call this “class promiscuity” when one class uses methods and data that are internal to another class.
3. Message Chaining — Imagine a series of classes that cyclically depend on each other to perform a certain function. If one fails, the entire chain fails.
4. Middle Man — A class that delegates work to another class should not exist.

Conclusion

The goal of this article was not to condemn anyone, as most of us are guilty of the above. Sometimes resource and time constraints force us to choose quality over quantity. Just as the law of entropy dictates that disorder is the natural state of order, every system in the universe will naturally gravitate toward a state of total depravity. The same applies to your codebase; if you don’t apply a conscious effort to refactor what may be perceived as a smell, you will often find yourself in a state of discontent.