← Back to System Internals

Code Smells — A Practical Refactoring Guide

A working reference to the classic code smells — the surface symptoms of deeper design trouble — and the refactorings that clear them, each shown as wrong-versus-correct code.

A code smell is a hint. It is a pattern in your source that, more often than not, signals a design weakness worth addressing. The value of the vocabulary is speed: once a team shares names like “Feature Envy” or “Shotgun Surgery,” a reviewer can point at a problem in two words instead of a paragraph, and everyone knows roughly what fix to reach for. This guide walks the five patterns of smells and, for every individual smell, gives a short summary and a side-by-side wrong/correct example so the fix is concrete.

What is a code smell

A code smell is a surface symptom that usually points to a deeper design problem. It is not a bug — the program can compile, pass its tests, and ship with every smell in this guide intact. What a smell tells you is that the code will be harder than it should be to read, change, or extend later. It is a prompt to look closer, not a verdict.

The critical caveat: smells are heuristics, not rules. A long method in a hot loop may be exactly right; a tiny class that looks “lazy” may earn its keep as a domain concept. The discipline is to notice the smell, ask whether it is actually hurting you, and refactor only when the answer is yes. Treat the catalog as a checklist for your intuition, never as a linter that must be obeyed.

Pattern 1: Bloaters

Bloaters are code, methods, and classes that have grown so large they are hard to work with. They rarely start big — they accrete, one “just one more line” at a time, until nobody wants to touch them.

1.1 Long Method

What it is: a function doing too many things at too many levels of detail.
Why it matters: it is hard to read, hard to test in parts, and hides reusable logic. The fix is Extract Method — pull each coherent chunk into a well-named helper until the original reads like a summary.

def print_invoice(order):
    # validate
    if not order.items:
        raise ValueError("empty order")
    # calculate
    total = 0
    for item in order.items:
        total += item.price * item.qty
    if order.customer.is_gold:
        total *= 0.85
    # format
    print(f"Invoice for {order.customer.name}")
    print(f"Total: ${total:.2f}")

def print_invoice(order):
    _validate(order)
    total = _order_total(order)
    _render(order.customer, total)


def _validate(order):
    if not order.items:
        raise ValueError("empty order")


def _order_total(order):
    subtotal = sum(i.price * i.qty for i in order.items)
    return subtotal * 0.85 if order.customer.is_gold else subtotal


def _render(customer, total):
    print(f"Invoice for {customer.name}")
    print(f"Total: ${total:.2f}")

1.2 Large Class

What it is: a class with too many fields and unrelated responsibilities.
Why it matters: it becomes a magnet for change and impossible to reason about. The fix is Extract Class — split each cohesive responsibility into its own collaborator.

class User:                     # does everything
    def __init__(self, name, email):
        self.name = name
        self.email = email
    def save(self):             # persistence
        db.insert("users", self.__dict__)
    def check_password(self, pw):   # auth
        return bcrypt.verify(pw, self.hash)
    def send_welcome(self):     # notifications
        smtp.send(self.email, "Welcome!")

@dataclass
class User:
    name: str
    email: str

class UserRepository:
    def save(self, u): db.insert("users", asdict(u))

class Authenticator:
    def check(self, u, pw): return bcrypt.verify(pw, u.hash)

class Mailer:
    def welcome(self, u): smtp.send(u.email, "Welcome!")

1.3 Long Parameter List

What it is: four, five, six parameters that are hard to remember and easy to transpose.
Why it matters: call sites are error-prone and the signature churns. The fix is Introduce Parameter Object so related arguments travel together.

def book_flight(origin, dest, depart, ret,
                adults, children,
                seat_class, currency):
    ...

book_flight("SFO", "JFK", d1, d2,
            2, 1, "economy", "USD")

@dataclass
class FlightQuery:
    origin: str
    dest: str
    depart: date
    ret: date | None = None
    adults: int = 1
    children: int = 0
    seat_class: str = "economy"
    currency: str = "USD"

def book_flight(q: FlightQuery):
    ...

1.4 Data Clumps

What it is: the same few data items appearing together in many places (say start, end, tz).
Why it matters: the grouping is a hidden concept, and logic over it gets duplicated. The fix is Extract Class — make the clump a type with its own behavior.

def overlaps(start1, end1, tz1,
             start2, end2, tz2):
    ...

def duration(start, end, tz):
    ...
# the trio travels together everywhere

@dataclass(frozen=True)
class TimeRange:
    start: datetime
    end: datetime
    tz: str

    def overlaps(self, other: "TimeRange") -> bool:
        ...
    def duration(self):
        return self.end - self.start

1.5 Primitive Obsession

What it is: using raw strings and ints to model concepts that deserve a type (a currency amount as a bare float).
Why it matters: validation and behavior get scattered, and illegal states are representable. The fix is to replace the primitive with a small value object.

def add_tax(amount, currency, rate):
    # amount is a bare float, currency a loose str
    # nothing stops mixing USD with EUR,
    # or float rounding drift on money
    return round(amount * (1 + rate), 2)

price = 19.99
total = add_tax(price, "USD", 0.08)

@dataclass(frozen=True)
class Money:
    amount: Decimal
    currency: str

    def add_tax(self, rate: Decimal) -> "Money":
        return Money(self.amount * (1 + rate),
                     self.currency)

total = Money(Decimal("19.99"), "USD").add_tax(Decimal("0.08"))

Pattern 2: Object-Orientation Abusers

These smells are incomplete or incorrect uses of object-oriented mechanisms — polymorphism, inheritance, interfaces — where the code fights the paradigm instead of leaning on it.

2.1 Switch Statements

What it is: a switch or if/elif chain branching on a type code, especially when the same chain reappears elsewhere.
Why it matters: every new type forces edits in every chain. The fix is Replace Conditional with Polymorphism.

def pay(employee):
    if employee.kind == "hourly":
        return employee.hours * employee.rate
    elif employee.kind == "salaried":
        return employee.salary / 12
    elif employee.kind == "commissioned":
        return employee.base + employee.sales * 0.1
    raise ValueError(employee.kind)

class Employee:
    def pay(self) -> Decimal:
        raise NotImplementedError

class Hourly(Employee):
    def pay(self): return self.hours * self.rate

class Salaried(Employee):
    def pay(self): return self.salary / 12

class Commissioned(Employee):
    def pay(self): return self.base + self.sales * 0.1

2.2 Refused Bequest

What it is: a subclass that inherits methods or data it does not want and cannot use.
Why it matters: it signals the inheritance is wrong and sets a trap for callers. The fix is to restructure the hierarchy (or replace inheritance with delegation) so a class only inherits what it uses.

class Bird:
    def fly(self): ...
    def lay_egg(self): ...

class Penguin(Bird):
    def fly(self):
        # inherited but nonsensical
        raise NotImplementedError("can't fly")

class Bird:
    def lay_egg(self): ...

class FlyingBird(Bird):
    def fly(self): ...

class Penguin(Bird):        # only inherits what it uses
    def swim(self): ...

2.3 Temporary Field

What it is: a field that is only set and meaningful during part of an object’s life, sitting empty the rest of the time.
Why it matters: readers cannot tell when it is valid, inviting bugs. The fix is to make it a local, or Extract Class for the field and its logic.

class Calculator:
    def compute(self, data):
        self._scratch = self._prepare(data)  # temp field
        return self._finish()

    def _finish(self):
        # only valid mid-compute
        return sum(self._scratch)

class Calculator:
    def compute(self, data):
        scratch = self._prepare(data)  # local, not a field
        return self._finish(scratch)

    def _finish(self, scratch):
        return sum(scratch)

2.4 Alternative Classes with Different Interfaces

What it is: two classes that do the same job but expose different method names and signatures.
Why it matters: callers must special-case which one they hold, so they cannot be swapped. The fix is to align the interfaces (Rename Method, Move Method) behind a common contract.

class FileLog:
    def write_line(self, msg): ...

class CloudLog:
    def push(self, text): ...   # same job, different API

# callers must know which one they have

class Log(Protocol):
    def log(self, msg: str) -> None: ...

class FileLog:
    def log(self, msg): ...

class CloudLog:
    def log(self, msg): ...      # now interchangeable

Pattern 3: Change Preventers

These smells make change expensive: one conceptual change forces edits in many unrelated places, or in the wrong number of places. They are the most direct tax on velocity.

3.1 Divergent Change

What it is: one class that changes for many different reasons (the database and the tax rules and the report layout all touch it).
Why it matters: unrelated changes collide in one file. The fix is Extract Class so each reason to change lives in one place.

class SalesReport:
    def load(self):
        db.query(...)      # changes with the DB
    def compute(self):
        ...                # changes with the math
    def render_pdf(self):
        ...                # changes with the layout

class SalesData:       # only the data source
    def load(self): db.query(...)

class SalesMetrics:    # only the math
    def compute(self, data): ...

class SalesPdf:        # only the layout
    def render(self, metrics): ...

3.2 Shotgun Surgery

What it is: the mirror image — one change forces small edits scattered across many classes.
Why it matters: it is easy to miss a spot, and every change is a hunt. The fix is Move Method/Move Field to pull the scattered responsibility into one home.

class Cart:
    def total(self): return self.sub * 1.08
class Invoice:
    def total(self): return self.sub * 1.08
class Quote:
    def total(self): return self.sub * 1.08
# change the tax rate -> edit every class

class Tax:
    RATE = 0.08
    @staticmethod
    def apply(amount):
        return amount * (1 + Tax.RATE)

class Cart:
    def total(self): return Tax.apply(self.sub)
class Invoice:
    def total(self): return Tax.apply(self.sub)

3.3 Parallel Inheritance Hierarchies

What it is: every time you add a subclass in one hierarchy you must add a matching subclass in another.
Why it matters: the duplication grows without bound. The fix is to let one hierarchy own the behavior so the parallel one disappears.

class Circle(Shape): ...
class Square(Shape): ...

class CircleRenderer(Renderer): ...
class SquareRenderer(Renderer): ...
# add a shape -> add a matching renderer

class Shape:
    def render(self, canvas): ...  # shape draws itself

class Circle(Shape):
    def render(self, canvas): canvas.circle(...)

class Square(Shape):
    def render(self, canvas): canvas.rect(...)

Pattern 4: Dispensables

Dispensables are things whose absence would make the code cleaner. They add volume and reading cost without adding value.

4.1 Duplicate Code

What it is: the same structure in more than one place — the single most common smell.
Why it matters: a fix must be applied everywhere, and one copy always gets missed. The fix is Extract Method (or Pull Up Method across subclasses).

def area_cm(w_mm, h_mm):
    return (w_mm / 10) * (h_mm / 10)

def perimeter_cm(w_mm, h_mm):
    return 2 * (w_mm / 10 + h_mm / 10)
# the mm->cm conversion is copied

def _cm(mm):
    return mm / 10

def area_cm(w_mm, h_mm):
    return _cm(w_mm) * _cm(h_mm)

def perimeter_cm(w_mm, h_mm):
    return 2 * (_cm(w_mm) + _cm(h_mm))

4.2 Dead Code

What it is: a variable, parameter, branch, method, or class no longer used.
Why it matters: it misleads readers into thinking it matters and inflates maintenance. The fix is to delete it — version control remembers it if you ever need it back.

def price(order, legacy_flag=False):  # never used
    discount = 0          # computed, never read
    total = order.subtotal
    if False:             # unreachable
        total *= 0.9
    return total

def price(order):
    return order.subtotal

4.3 Lazy Class

What it is: a class that no longer does enough to justify its existence.
Why it matters: it adds indirection and a file to maintain for no benefit. The fix is Inline Class, folding it into its only caller.

class Name:
    def __init__(self, value):
        self.value = value
    def get(self):
        return self.value   # adds nothing over a str

user.name = Name("Ada")
print(user.name.get())

user.name = "Ada"       # just use the string
print(user.name)

4.4 Speculative Generality

What it is: abstractions, hooks, and parameters built for a future that never came.
Why it matters: the unused flexibility is pure cost — more code, more indirection, harder reading. The fix is to collapse it. YAGNI.

class AbstractExporterFactory:   # only ever makes CSV
    def create(self, kind="csv", **future_opts):
        return CsvExporter()

exporter = AbstractExporterFactory().create()

exporter = CsvExporter()
# add a factory when a 2nd format
# actually appears, not before

4.5 Data Class

What it is: a class that is only fields and getters/setters, with all the behavior that operates on it living elsewhere.
Why it matters: logic drifts away from the data it needs (often causing Feature Envy). The fix is Move Method to bring behavior onto the data.

@dataclass
class Rectangle:
    width: float
    height: float

def area(r):            # behavior lives away
    return r.width * r.height

@dataclass
class Rectangle:
    width: float
    height: float

    def area(self):     # behavior on its data
        return self.width * self.height

4.6 Comments as Deodorant

What it is: a block of comments explaining what a tangle of code does, masking a smell.
Why it matters: comments rot and the tangle stays. The fix is to Extract Method named after the comment; keep comments that explain why, not what.

# check if user is an active premium subscriber
if (u.plan == "premium"
        and u.status == "active"
        and u.expires > today()):
    grant_access()

if is_active_premium(u):
    grant_access()

def is_active_premium(u):
    return (u.plan == "premium"
            and u.status == "active"
            and u.expires > today())

Pattern 5: Couplers

Couplers are smells of excessive coupling between classes — or, in the case of Middle Man, of coupling avoided so aggressively that a class stops pulling its weight.

5.1 Feature Envy

What it is: a method more interested in another class’s data than its own, reaching across to grab several of its fields.
Why it matters: it couples the two classes and splits behavior from data. The fix is Move Method to put the behavior where the data lives.

class Order:
    def total(self, customer):
        # reaches into customer for everything
        discount = customer.tier_discount
        credit = customer.loyalty_credit
        return (self.subtotal * (1 - discount)
                - credit)

class Customer:
    def net_of(self, subtotal):
        return (subtotal * (1 - self.tier_discount)
                - self.loyalty_credit)

class Order:
    def total(self, customer):
        return customer.net_of(self.subtotal)

5.2 Inappropriate Intimacy

What it is: two classes reaching deep into each other’s private internals.
Why it matters: neither can change without breaking the other. The fix is Move Method/Move Field to shrink the shared surface and talk through interfaces.

class Customer:
    def __init__(self):
        self._orders = []

class Order:
    def link(self, c):
        c._orders.append(self)      # into privates
        self._tier = c._tier

class Customer:
    def add_order(self, o):
        self._orders.append(o)      # own your data

class Order:
    def link(self, c):
        c.add_order(self)           # via interface

5.3 Message Chains

What it is: a client walking a long path like a.b().c().d().
Why it matters: the caller is coupled to the whole navigation structure, so any intermediate change breaks it. The fix is Hide Delegate — give the first object a method that hides the chain.

city = (order.get_customer()
             .get_address()
             .get_city())
# coupled to the whole path

class Order:
    def customer_city(self):
        return self._customer.city()   # hide delegate

city = order.customer_city()

5.4 Middle Man

What it is: a class where most methods just delegate to another class, adding nothing.
Why it matters: it is indirection for its own sake. The fix is Remove Middle Man and let clients talk to the real object.

class Manager:
    def __init__(self, worker):
        self._w = worker
    def do_task(self):
        return self._w.do_task()
    def report(self):
        return self._w.report()
    # adds nothing but forwarding

# drop the middle man; use the worker directly
worker = Worker()
worker.do_task()
worker.report()

How to use smells well

Knowing the catalog is the easy part; using it without wrecking a working system is the skill. The loop below keeps refactoring safe — behavior never changes, only structure.

• Smells guide; they don’t govern. Use them to find candidates for improvement, then judge each one on its merits. Not every smell needs a fix today.
• Tests protect. Refactoring means changing structure without changing behavior — a test suite is what lets you prove you did exactly that.
• Refactor in small steps. Make one named refactoring at a time and keep the code compiling and green throughout. Small steps are easy to reason about and easy to undo.
• Don’t refactor without coverage. If the area has no tests, add characterization tests first. Refactoring blind is just editing and hoping.
• Follow the Boy Scout rule. Leave each file a little cleaner than you found it. Small, opportunistic cleanups compound faster than big rewrites.
• Review for smells. Bring the shared vocabulary into code review — naming a smell is a fast, low-heat way to flag a design concern.

Summary