← CatZep/Engineering Guides/Reference/Engineering Craft & Process

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.

Concepts explained in this guide

Read these definitions first, or use the linked terms as you go. The examples and limitations are local to this guide; companion pages are optional.

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.

Worked example: extract validation without changing behavior

Use this small example to connect the concept to a concrete decision.

Run and change the example

Download guide_labs.py. Python 3.10+, standard library only. In its directory, run:

python3 guide_labs.py refactoring
python3 guide_labs.py test

The lab compares two implementations over valid and invalid inputs, including exception behavior.

The program tests its stated finite contracts. It is an educational model, not an implementation of the named production product or a proof of unmodeled distributed faults.

Summary