Steven's Knowledge

Code Smells

A diagnostic vocabulary for code that likely needs refactoring

Code Smells

A code smell is a structural pattern that often, but not always, indicates a problem. Smells are diagnostic, not prescriptive — they suggest a closer look, not a fix.

The point of a shared vocabulary is to make code review faster and more concrete. "I think this has Feature Envy" is a more useful comment than "this feels off."

Within a Function

Long Function

A function that does not fit on a screen, or that contains several distinct ideas. Extract sub-functions named for what each section accomplishes; the original function then reads as a sequence of intentions.

Long Parameter List

More than three or four parameters. Group related parameters into an object, replace a flag with two functions, or split the function so each variant takes only what it needs.

Mysterious Name

A name from which the reader cannot predict the value or behavior. The fix — renaming — is the cheapest, highest-return refactoring available, and the easiest to defer until it is too late.

Comments Compensating for Bad Code

A long comment explaining what a block of code does is a candidate for extraction: the comment becomes the function name and the block becomes the body.

Within a Class or Module

Large Class

A class with many fields and methods covering several responsibilities. Ask: can the class be described in a single sentence without "and"? If not, extract sub-classes around clusters of related fields and methods.

Data Class

A class with fields and accessors but no behavior. Behavior that operates on these fields probably lives somewhere else and should be moved closer to the data — or the class should be replaced by a value type if it is genuinely just a record.

Primitive Obsession

Modeling domain concepts with primitive types: string for an email address, int for a quantity-with-unit, string for an enum-like set. Replace primitives with small types that name the concept and enforce its constraints.

Lazy Element

A class or function that does not justify its own existence — a thin wrapper over a single call, a class with one field. Inline it.

Speculative Generality

Abstractions, base classes, or hooks that exist for needs no caller actually has. Remove them; reintroduce when the need is concrete. Speculative generality is the one cost of YAGNI ("You Aren't Gonna Need It") that compounds.

Temporary Field

An instance field that is meaningful only in some states or some methods. Often a sign that the methods using it should be extracted into their own class.

Coupling

Feature Envy

A method that talks to another object's data more than to its own. Move the method to where the data lives.

Inappropriate Intimacy

Two classes that read or modify each other's internals. Either merge them, extract a third class to hold the shared work, or replace inheritance with delegation.

Message Chains

A long sequence of calls — a.getB().getC().getD().doSomething() — that exposes the chain's intermediate structure to the caller. Hide the chain behind a method on a (or, depending on intent, on b).

Middle Man

A class whose methods almost all delegate to another. The indirection adds cost without value; let callers talk directly to the underlying class, or absorb the methods that do add value.

Insider Trading

Modules that share data through back channels — global state, shared mutable singletons, side-channel files — instead of through their stated interfaces. Make the dependency explicit or eliminate it.

Change Patterns

These smells are visible only over time, not in a single snapshot:

Divergent Change

One module is changed for many unrelated reasons. The module is mixing concerns; split it along the axes of change.

Shotgun Surgery

One conceptual change requires edits in many places. The opposite of divergent change: a concern is scattered. Consolidate the related code.

Parallel Inheritance Hierarchies

Adding a subclass to one hierarchy forces adding a subclass to a parallel one. The two hierarchies should be merged or related by composition.

Inheritance

Refused Bequest

A subclass inherits methods or fields it does not want. Either the inheritance relationship is wrong, or the parent should be split.

Switch on Type

Repeated switch or if/else chains over a type code, scattered across the codebase. Replace with polymorphism — when there is only one such switch, inheritance is overkill, but when the same switch shape repeats in several places, polymorphism eliminates the duplication.

Working with the Vocabulary

Smells are heuristics, not laws

Each smell has counter-examples. A short class is not always a Lazy Element; sometimes it is exactly the right size. The smell flags a place to think; it does not mandate a fix.

Stable code with smells is often fine

Code that has not changed in a year and is unlikely to change soon does not pay back refactoring effort. Apply smells where change is happening.

A smell plus an upcoming change is the strongest signal

The combination — "this code has Feature Envy and I am about to add a related feature" — is when refactoring has the highest return.

Use smells in code review, not just in cleanup

The vocabulary is most valuable as a discussion tool. A reviewer pointing out "Long Parameter List" gives the author a concrete handle to act on, in a way that "this is hard to read" does not.

On this page