Skip to content
FollowtheDamnCode

Smells: Object-Orientation Abusers

OO features used half-heartedly or wrongly — repeated type switches, fields that are only sometimes set, subclasses that refuse their inheritance.

Big idea

OO abusers are smells where code uses object-oriented features incompletely or incorrectly: switching on types instead of using polymorphism, fields that only mean something some of the time, subclasses that don't want what they inherit, and classes that do the same job under different names. Each one fights the language instead of letting it help.

The abusers

SmellWhat it looks likeUsual fix
Switch StatementsThe same if kind == … / match on a type code in several placesReplace Conditional with Polymorphism (Simplifying Conditionals); Strategy or State
Temporary FieldA field set only during one algorithm and None the rest of the timeExtract Class for that algorithm; or pass values as parameters
Refused BequestA subclass that overrides inherited methods to raise or do nothingReplace Inheritance with Delegation (Generalization); fix the hierarchy (Liskov)
Alternative Classes with Different InterfacesEmailSender.send_mail(to, body) and SmsSender.dispatch(body, number) doing the same jobRename Method, Extract Interface so they're interchangeable

Think of it like

A hospital where every department keeps its own paper form for "patient arrives", each with a different layout, and the receptionist has a binder of instructions — "if cardiology, fill in form C7 in blue ink; if radiology…". Add a department and the binder gets another page, and the receptionist must remember to read it. A standard intake form that each department extends is the polymorphic fix.

Why repeated switches hurt

One match on a type code is fine. The smell is the second and third copy: price() switches on plan type, limits() switches on plan type, badge() switches on plan type. Adding a plan means finding every switch — and the compiler (or Python) won't tell you which one you missed.

Three switches become one class per plan

Hands on: the switch someone forgot

A new "team" plan was added to the price switch but not the seat-limit switch, so team accounts are capped at the free plan's limit. Refactor so each plan's facts live together (a class per plan, or one object per plan), and the output matches.

Fix it: two switches drifted apart
def monthly_price(plan):
    if plan == "free":
        return 0
    elif plan == "team":
        return 40
    elif plan == "enterprise":
        return 300

def seat_limit(plan):
    if plan == "enterprise":
        return 1000
    else:
        return 3                       # nobody remembered to add "team" here

for plan in ["free", "team", "enterprise"]:
    print(f"{plan:10} {monthly_price(plan):4}/mo  up to {seat_limit(plan)} seats")
output
free          0/mo  up to 3 seats
team         40/mo  up to 25 seats
enterprise  300/mo  up to 1000 seats

Run and edit it in the app.

Now adding a plan means writing one class, and it's impossible to give it a price but forget its seats.

Hands on: a temporary field

RouteFinder stores _visited as a field that are only meaningful during one search; a second search reuses stale values. Make the search's working state local (or its own object) so each call starts fresh.

Fix it: state left over from the last call
class RouteFinder:
    def __init__(self, roads):
        self.roads = roads
        self._visited = set()          # only meaningful during find()

    def find(self, start, goal):
        if start == goal:
            return True
        self._visited.add(start)
        return any(self.find(n, goal) for n in self.roads.get(start, []) if n not in self._visited)


finder = RouteFinder({"A": ["B"], "B": ["C"], "C": []})
print("A->C:", finder.find("A", "C"))
print("A->C again:", finder.find("A", "C"))
output
A->C: True
A->C again: True

Run and edit it in the app.

The first search left A and B in _visited, so the second identical search thinks it has already been everywhere. A field that's only meaningful during one call is a bug waiting for the second call.

class Ostrich(Bird) overrides fly() to raise NotImplementedError. Which smell?

PdfReport.render_pdf() and CsvReport.write_csv_file() do the same job with different names. What's the fix?

How it relates

Key terms

  • Switch statements (smell) — The same type-based conditional repeated in several places.
  • Temporary field — A field that's only set and meaningful during part of an object's life.
  • Refused bequest — A subclass that doesn't want, or disables, what it inherits.
  • Type code — A value (string, int, enum) used to decide behaviour that could live in classes.