Reducing Code Smells & Anti-Patterns

Overview

A code smell is a surface-level signal that something in the design may be wrong. Smells are not bugs — the code runs — but they indicate structural problems that make code harder to understand, harder to change, and more likely to develop bugs over time. The term comes from Martin Fowler's Refactoring (1999) and has become standard vocabulary in software engineering.

This page catalogs the most common smells and anti-patterns, explains why they matter, and points to how to address them. For the techniques used to eliminate smells, see Refactoring Techniques.

Why It Matters

Smells compound over time. A slightly-too-large function gets one more parameter added per quarter. A slightly-too-coupled module gets one more dependency per sprint. The problem grows nonlinearly, and the refactoring cost grows with it. Identifying smells early keeps the cost of fixing them low.

Smells predict future bugs. Dense, tangled code is harder to reason about, which means engineers make incorrect assumptions about it. Incorrect assumptions become bugs. Studies of large codebases consistently show that complex, highly-coupled modules have significantly higher defect rates than simpler ones.

Naming smells is useful. When a team has shared vocabulary for common problems, code review improves. "This function has too many responsibilities" is less precise than "this is a God Object, let's extract the persistence logic." Naming the smell clarifies the problem and points toward the solution.

Taxonomy of Common Smells

Smells cluster into categories based on what structural problem they indicate.

Bloaters — things that have grown too large

Long Function A function that does too much. Functions should fit on a screen and do one thing. If you need to scroll to read a function, or if it handles multiple concerns, extract sub-functions.

Signal: the function has more than one level of abstraction. Setup code, business logic, and persistence all in the same block.

Long Parameter List A function that takes 4+ parameters. Beyond three parameters, callers have to count positions; the function likely handles too many concerns; and adding a new requirement means changing the signature everywhere.

Fix: introduce a parameter object. Group related parameters into a typed struct or interface.

Large Class / God Object A class or module that knows too much and does too much. It has dozens of methods, touches data it shouldn't, and becomes the go-to place for new code because "it's already there."

Fix: identify distinct responsibilities and extract them into separate classes or modules. Each module should have one reason to change.

Data Clumps The same group of 3–4 variables always appear together in function signatures or class fields. If you can't remove one without the others becoming meaningless, they're really one thing pretending to be several.

Fix: introduce a parameter object or a value type that holds them together.

Change Preventers — things that make change expensive

Divergent Change A single module that must be modified for unrelated reasons. Adding a new payment method touches the same file as adding a new report format. The module has multiple responsibilities.

Fix: separate the responsibilities into distinct modules. One module should change for one reason.

Shotgun Surgery The inverse: a single logical change requires edits across many unrelated modules. Fixing a date format means touching 12 different files.

Fix: consolidate related logic. Move the scattered pieces to a single place so that a future change touches one file, not twelve.

Parallel Inheritance Hierarchies Every time you add a subclass to one hierarchy, you must add a matching subclass to another. The two hierarchies are implicitly coupled.

Fix: merge the hierarchies or use composition instead of inheritance to associate behaviors.

Dispensables — things that add no value

Dead Code Functions, variables, imports, or entire modules that are never called. Dead code accumulates silently and misleads future readers into thinking it is relevant.

Fix: delete it. Version control preserves the history if it's ever needed.

Speculative Generality Abstractions and hook points built for future requirements that don't exist yet. "We might need to support multiple payment providers someday" — so the code has a factory, an interface, a registry, and a strategy pattern for a single payment provider.

Fix: delete the generalization. Implement what is needed now. Generalize when the second use case actually arrives.

Duplicate Code The same logic in two or more places. When the logic changes, both places must be updated. One usually gets missed.

Fix: Extract the shared logic into a single function and call it from both places. The threshold for extraction is low: if you see the same block twice, it belongs in its own function.

Lazy Class / Freeloader A class or module that does so little it barely justifies existing. Often the result of a refactoring that left an empty shell, or a premature abstraction that never got filled in.

Fix: inline it into its caller, or merge it into a closely related module.

Couplers — things that create unwanted dependencies

Feature Envy A function that is more interested in the data of another class than in its own. It accesses fields of another object repeatedly to do its work.

Signal: order.customer.address.city appears 5 times in a function that lives on InvoiceRenderer.

Fix: move the function to the class whose data it uses. Or, have that class expose a higher-level method that encapsulates the logic.

Inappropriate Intimacy Two classes that know too much about each other's internals — accessing private fields directly (via public accessors that shouldn't be public), calling internal methods, or sharing mutable state.

Fix: tighten the interface between them. The classes should interact through a well-defined, minimal API.

Message Chains Code that navigates a chain of object accesses: a.getB().getC().getD().doSomething(). This creates a tight coupling to the object graph — if any link in the chain changes, the caller breaks.

Fix: apply the Law of Demeter: a module should only talk to its immediate dependencies. Have the intermediate object expose the behavior you need, rather than exposing its internals.

Temporal Coupling Two or more operations that must be called in a specific order to work correctly, where nothing in the type system or API enforces that order. The constraint exists only in the caller's memory or in documentation nobody reads.

// Temporal coupling — order is required but invisible from the types
const service = new PaymentService();
service.loadConfig(); // must be first
service.connect(); // must be second
service.processPayment(order); // silently fails if the above weren't called

If processPayment is called without connect, the failure may not be immediate — it may manifest as wrong behaviour far from the call site, or only under certain timing conditions in async code.

Fix: make the dependency explicit through the type system. Have each step return the precondition the next step requires:

const config = await PaymentService.loadConfig();
const connection = await config.connect();
await connection.processPayment(order);

Or hide the sequence behind a single factory that enforces it internally: PaymentService.create(). If sequential initialisation is genuinely unavoidable, the type signature should make it impossible to skip a step.

Signal to look for: init(), setup(), connect(), or initialize() methods that must be called before "real" methods, or multi-step setup in test fixtures where the order of beforeEach blocks is load-bearing but non-obvious. Particularly common in event emitters, SDK initialisation, database connection wrappers, and stateful service clients.

Primitive Obsession Using primitive types (strings, integers) to represent domain concepts. A userId is a string. A currency is a string. An emailAddress is a string.

Fix: introduce value types. UserId, Currency, EmailAddress — even as thin wrappers — make types self-documenting, enable validation at the boundary, and prevent the classic getUserById(email, userId) argument-swap bug.

Anti-Patterns — higher-level design mistakes

Shotgun Approach to Error Handling Try-catch blocks scattered throughout business logic, each handling errors inconsistently. Some swallow errors silently, some log and continue, some rethrow. The result is unpredictable error propagation.

Fix: define a clear error handling strategy. In most cases: validate at the boundary, propagate errors up, handle them at the top level in one place.

Magic Numbers and Strings Literals embedded in logic with no explanation: if (retries > 3), status === "ACT", price * 1.08.

Fix: assign them to named constants with clear intent: MAX_RETRY_ATTEMPTS, ACTIVE_STATUS, SALES_TAX_RATE.

Boolean Trap A function that takes a boolean parameter to switch behavior: renderButton(label, true). The call site is unreadable — what does true mean?

Fix: split into two functions (renderPrimaryButton, renderSecondaryButton), or use an options object with a named field.

Arrow Code Deeply nested conditionals that push logic to the right edge of the screen. Also called "pyramid of doom."

// Arrow code
function process(order) {
  if (order) {
    if (order.items.length) {
      if (order.customer) {
        if (order.customer.isVerified) {
          // actual logic
        }
      }
    }
  }
}

Fix: invert the conditionals and return early. Each guard clause exits when a condition fails, leaving the happy path at the top level.

// Guard clauses
function process(order) {
  if (!order) return;
  if (!order.items.length) return;
  if (!order.customer) return;
  if (!order.customer.isVerified) return;
  // actual logic
}

How to Implement

Detection strategies

Automated metrics. Most linters can be configured to flag some smells directly: cyclomatic complexity thresholds, maximum function length, maximum file length, maximum parameter count. These aren't substitutes for judgment, but they surface candidates automatically.

Code review. Reviewers should name smells explicitly when they see them. A comment that says "this looks like a God Object — could we extract the persistence layer?" is more useful than "this class is doing too much."

Hotspot analysis. Files that are changed frequently (high churn) and have many authors often have higher defect rates and tend to accumulate smells. Run git log --oneline -- <file> to see how often a file changes. High-churn files are good candidates for attention.

Prioritization

Not all smells are equally costly to fix. Prioritize by:

1. Frequency of change — smells in files that change every sprint compound fastest. Fix those first.
2. Size of the affected surface — a God Object that dozens of modules depend on is more damaging than a smelly utility function called from one place.
3. Defect density — if a module has a history of bugs, its smells are probably contributing. Prioritize it.
4. Upcoming work — if a smelly module is about to be extended, refactor it before adding the extension, not after.

The Boy Scout Rule

Leave code slightly better than you found it. When you open a file to make a change, fix the obvious smell nearby if it takes less than 15 minutes. Don't try to refactor the whole file — just improve what you touch. Over time, this compounds into a meaningfully cleaner codebase without dedicated "cleanup sprints."

Tools & Templates

ESLint rules that catch smells

{
  "rules": {
    "complexity": ["warn", { "max": 10 }],
    "max-lines-per-function": ["warn", { "max": 50 }],
    "max-params": ["warn", { "max": 4 }],
    "max-depth": ["warn", { "max": 4 }],
    "no-unused-vars": "error"
  }
}

Finding high-churn files

# Top 20 most frequently changed files in the last 6 months
git log --since="6 months ago" --name-only --format="" | \
  sort | uniq -c | sort -rn | head -20

Finding duplicate code

Tools like jscpd (JavaScript) or PMD CPD (multi-language) detect copy-pasted blocks across a codebase:

pnpm dlx jscpd --min-tokens 50 --reporters console ./src

Common Pitfalls

Treating all smells as equally urgent. A primitive-obsessed utility function used in two places is not the same as a God Object at the center of your architecture. Triage by impact before spending time.

Smell-hunting as procrastination. It is possible to spend weeks identifying and cataloging smells without shipping anything. Detection should feed into prioritized refactoring work — not become an end in itself.

Eliminating a smell without understanding why it exists. Some smells are symptoms of genuine constraints: a long function that handles a genuinely complex algorithm, a broad module because the domain itself isn't decomposable cleanly. Understand before eliminating.

Introducing new smells during refactoring. Extracting a function often creates a new function that is too small and does too little (Lazy Class / Freeloader). Introducing a parameter object can create a Data Clump at a higher level. Every refactoring step should be checked: did I introduce a new smell while fixing the old one?