Code Smells
Status: Complete
Category: Clean Code
Default enforcement: Advisory
Author: PushBackLog team
Tags
- Topic: clean-code, quality
- Skillset: backend, frontend, fullstack
- Technology: generic
- Stage: execution, review
Summary
Code smells are surface-level symptoms in source code that indicate deeper structural problems. They are not bugs — the code may be correct — but they signal that the design is working against the engineer, increasing the cost of change, adding cognitive load, and creating brittle seams where future changes are likely to introduce defects. Recognising and naming code smells is the first step toward systematic refactoring.
Rationale
Smells as diagnostic signals
Martin Fowler and Kent Beck’s taxonomy of code smells (from Refactoring) provides a shared vocabulary for discussing code quality. Without named smells, engineers either say vague things like “this feels messy” or say nothing at all. With named smells, a reviewer can say “this is a Feature Envy — calculateDiscount is reaching into three Customer fields and would better belong on Customer” and propose a concrete, targeted refactoring.
Technical debt lives in smells
Smells are how technical debt becomes visible before it becomes a maintenance crisis. A codebase that accumulates smells without remediation becomes increasingly expensive to change — not because any single smell is catastrophic, but because they compound. Long methods make it hard to test anything in isolation. Divergent changes mean one requirement touches eight files. Data clumps mean the same three parameters appear in fifteen function signatures. The combination is a codebase where engineers dread touching anything because the blast radius of every change is large and unpredictable.
Guidance
Common code smells
Bloaters — things that have grown too large
| Smell | Description | Refactoring |
|---|---|---|
| Long Method | Function too long to understand at a glance (> 20 lines is a heuristic) | Extract Method |
| Large Class | Class with too many responsibilities, fields, or methods | Extract Class, Extract Subclass |
| Long Parameter List | Function with four or more parameters | Introduce Parameter Object, Preserve Whole Object |
| Data Clumps | The same two or three values always appear together | Extract Class into a Value Object |
| Primitive Obsession | Using primitives instead of small objects for domain concepts (money as number, status as string) | Replace Primitive with Object |
Object-orientation abusers — misuse of OO constructs
| Smell | Description | Refactoring |
|---|---|---|
| Switch Statements | Same switch or if-else chain appearing in multiple places | Replace Conditional with Polymorphism |
| Temporary Field | Instance variable only set under certain conditions | Introduce Null Object, Extract Class |
| Refused Bequest | Subclass inherits parent interface but only uses part of it | Replace Inheritance with Delegation |
| Alternative Classes with Different Interfaces | Two classes doing the same thing with different method names | Rename Method, Extract Superclass |
Change preventers — changes that ripple unexpectedly
| Smell | Description | Refactoring |
|---|---|---|
| Divergent Change | One class changes for many different reasons | Extract Class per reason-to-change |
| Shotgun Surgery | One change requires small edits scattered across many classes | Move Method, Move Field |
| Parallel Inheritance Hierarchies | Adding a subclass in one hierarchy forces a new subclass in another | Move Method, Move Field |
Couplers — excessive interaction between classes
| Smell | Description | Refactoring |
|---|---|---|
| Feature Envy | A method that uses more data from another class than its own | Move Method |
| Inappropriate Intimacy | Two classes know too much about each other’s internals | Move Method, Move Field, Change Bidirectional to Unidirectional |
| Message Chains | a.getB().getC().getD().doThing() — the Law of Demeter violated | Extract Method, Hide Delegate |
| Middle Man | A class that does nothing but delegate to another | Inline Method, Remove Middle Man |
Dispensables — things that should not exist
| Smell | Description | Refactoring |
|---|---|---|
| Comments | Comments that exist to explain what unclear code does (not why) | Extract Method with a good name |
| Duplicate Code | Same or similar logic in multiple places | Extract Method, Pull Up Method |
| Lazy Class | A class that does too little to justify existing | Inline Class, Collapse Hierarchy |
| Data Class | A class with only getters/setters and no behaviour | Move Method into the class |
| Dead Code | Unreachable or unused code | Delete it |
| Speculative Generality | Abstraction created for hypothetical future requirements (YAGNI violation) | Inline Class, Remove Parameter |
Using smells in code review
Code review comments citing a named smell are more actionable and less personal than vague quality feedback:
# Too vague
"This method feels too big"
# More actionable
"This looks like a Long Method — the payment processing and notification logic
could be extracted into `ChargeCard` and `NotifyOrderPlaced` respectively,
with the calling method reading like a summary."Name the smell, describe the specific instance, and suggest the refactoring.
Metrics that surface smells
- Cyclomatic complexity (> 10 in a single function is a smell) — detects complex branching (Long Method, Switch Statements)
- Lines of code per function (> 30 is a heuristic) — detects Long Method
- Number of parameters (> 3) — detects Long Parameter List
- Class coupling — detects Feature Envy, Inappropriate Intimacy
- Duplication rate — detects Duplicate Code
Most linting or static analysis tools can be configured to report these automatically as part of CI.