Skip to content
FollowtheDamnCode

Smells: Dispensables

Code you'd be better off without — duplicates, dead code, comments covering for unclear code, classes that do too little, and generality nobody uses.

Big idea

Dispensables are things whose absence would make the code cleaner and safer: duplicated logic, code that never runs, comments that apologise for confusing code, classes too small to justify themselves, and hooks for futures that never came. Every line is something to read, test, and keep consistent — so lines that earn nothing cost something.

The dispensables

SmellWhat it looks likeUsual fix
Duplicate CodeThe same logic in two places, slowly drifting apartExtract Method, Pull Up Method, Form Template Method
Comments (as a smell)A comment explaining what a confusing block doesExtract Method or Rename so the code says it; keep comments for why
Dead CodeUnreachable branches, unused functions, parameters nobody passesDelete it — version control remembers
Lazy ClassA class that barely does anything after earlier refactoringsInline Class
Data ClassOnly fields and getters/setters; all the logic about it lives elsewhereMove Method the behaviour onto it
Speculative GeneralityAbstract classes with one subclass, unused parameters, hooks "for later"Collapse Hierarchy, Inline Class, remove parameters (YAGNI)

Think of it like

Moving house. Packing is when you discover three identical phone chargers, a drawer of cables for devices you no longer own, and a label on a box that says "misc — important?". Throwing them out doesn't lose anything; it makes everything else easier to find.

Each dispensable and the refactoring that removes it

Comments: why vs what

# check if the user is eligible: over 18 and has verified email and not banned
if u.age > 18 and u.email_verified and not u.banned:

The comment restates the code — and the day someone changes > 18 to >= 18 it will be wrong. Name the idea instead: if user.is_eligible():. Keep comments that explain why ("the bank's API rejects amounts with more than 2 decimals, so round first") — code can't say that.

Hands on: duplicate code that drifted

Two reports compute VAT. One was fixed to round each line before summing (as the tax office requires); the other wasn't, so the two totals disagree by a cent. Extract the calculation into one function both use, keeping the output shown.

Fix it: two copies of the VAT rule
LINES = [19.99, 5.03, 7.33]

def invoice_vat():
    return round(sum(round(p * 0.2, 2) for p in LINES), 2)    # fixed: round per line

def summary_vat():
    return round(sum(p * 0.2 for p in LINES), 2)              # old copy

print(f"invoice VAT: {invoice_vat():.2f}")
print(f"summary VAT: {summary_vat():.2f}")
output
invoice VAT: 6.48
summary VAT: 6.48

Run and edit it in the app.

The docstring that survived is a why comment — the one thing code can't express.

Hands on: a data class with behaviour stranded elsewhere

Subscription is pure data; the rule for "is it active today?" is re-derived by each caller, and one caller forgot that the end date is inclusive. Move the rule onto the class so there's one definition.

Fix it: every caller re-implements the rule
class Subscription:
    def __init__(self, start_day, end_day):
        self.start_day, self.end_day = start_day, end_day   # end day is inclusive

def can_stream(sub, today):
    return sub.start_day <= today <= sub.end_day

def show_renew_banner(sub, today):
    return not (sub.start_day <= today < sub.end_day)       # forgot: inclusive

sub = Subscription(1, 30)
for day in (15, 30):
    print(f"day {day}: stream={can_stream(sub, day)} banner={show_renew_banner(sub, day)}")
output
day 15: stream=True banner=False
day 30: stream=True banner=False

Run and edit it in the app.

Which comment is worth keeping?

An abstract Exporter has exactly one subclass, CsvExporter, and has had for two years. Which smell?

How it relates

Key terms

  • Duplicate code — The same logic in more than one place, liable to drift apart.
  • Dead code — Code that can never run or whose result is never used.
  • Lazy class — A class that does too little to justify its existence.
  • Data class — A class with fields but no behaviour, whose logic lives in other classes.
  • Speculative generality — Abstractions and hooks built for needs that never arrived.