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
| Smell | What it looks like | Usual fix |
|---|---|---|
| Duplicate Code | The same logic in two places, slowly drifting apart | Extract Method, Pull Up Method, Form Template Method |
| Comments (as a smell) | A comment explaining what a confusing block does | Extract Method or Rename so the code says it; keep comments for why |
| Dead Code | Unreachable branches, unused functions, parameters nobody passes | Delete it — version control remembers |
| Lazy Class | A class that barely does anything after earlier refactorings | Inline Class |
| Data Class | Only fields and getters/setters; all the logic about it lives elsewhere | Move Method the behaviour onto it |
| Speculative Generality | Abstract 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.
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.
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.48Run 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.
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=FalseRun 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
- Duplicate Code and Speculative Generality are DRY and YAGNI violations.
- Data Class is the opposite of Encapsulation — and often causes Feature Envy elsewhere.
- Previous family: Change Preventers. Next: Couplers.
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.