Module 20: Anti-Patterns & Code Smells

Why this matters

Every module so far taught a principle or pattern by building toward it. This module runs the tape backward: given a class, recognize which principle it's violating, by name, fast — because "here's some code, what's wrong with it?" is the actual opening move in most LLD interviews, not "design something from scratch." Every anti-pattern below is a smell this track already has the vocabulary and the fix for — SRP (module 04), real OOP encapsulation (module 02), the Law of Demeter (module 05), value objects (module 03). This module's job is pattern-matching real code against that vocabulary, not introducing new mechanics.

Concepts

God Object

A class that accumulates unrelated responsibilities until it does almost everything — the most commonly cited LLD smell, and a direct SRP (module 04) violation wearing a different name.

# VIOLATION: God Object — one class doing validation, hashing, email, and persistence
class UserManagerBad:
    def __init__(self):
        self.users = {}

    def register(self, email: str, password: str) -> bool:
        if "@" not in email:                                   # validation
            return False
        hashed = f"hashed:{password}"                            # password hashing
        print(f"Sending welcome email to {email}")               # email
        self.users[email] = hashed                               # persistence
        return True

# FIXED: one focused class per responsibility, UserManager only coordinates
class EmailValidator:
    def is_valid(self, email: str) -> bool:
        return "@" in email

class PasswordHasher:
    def hash(self, password: str) -> str:
        return f"hashed:{password}"

class WelcomeEmailSender:
    def send(self, email: str):
        print(f"Sending welcome email to {email}")

class UserRepository:
    def __init__(self):
        self._users: dict[str, str] = {}

    def save(self, email: str, hashed_password: str):
        self._users[email] = hashed_password

class UserManager:                    # coordinates — doesn't do any of the work itself
    def __init__(self, validator: EmailValidator, hasher: PasswordHasher,
                 emailer: WelcomeEmailSender, repository: UserRepository):
        self._validator = validator
        self._hasher = hasher
        self._emailer = emailer
        self._repository = repository

    def register(self, email: str, password: str) -> bool:
        if not self._validator.is_valid(email):
            return False
        self._repository.save(email, self._hasher.hash(password))
        self._emailer.send(email)
        return True

// VIOLATION: God Object — one class doing validation, hashing, email, and persistence
public class UserManagerBad {
    private Dictionary<string, string> _users = new Dictionary<string, string>();

    public bool Register(string email, string password) {
        if (!email.Contains("@")) return false;                 // validation
        string hashed = $"hashed:{password}";                    // password hashing
        Console.WriteLine($"Sending welcome email to {email}");  // email
        _users[email] = hashed;                                   // persistence
        return true;
    }
}

// FIXED: one focused class per responsibility, UserManager only coordinates
public class EmailValidator {
    public bool IsValid(string email) => email.Contains("@");
}

public class PasswordHasher {
    public string Hash(string password) => $"hashed:{password}";
}

public class WelcomeEmailSender {
    public void Send(string email) => Console.WriteLine($"Sending welcome email to {email}");
}

public class UserRepository {
    private Dictionary<string, string> _users = new Dictionary<string, string>();
    public void Save(string email, string hashedPassword) => _users[email] = hashedPassword;
}

public class UserManager {                // coordinates — doesn't do any of the work itself
    private EmailValidator _validator;
    private PasswordHasher _hasher;
    private WelcomeEmailSender _emailer;
    private UserRepository _repository;

    public UserManager(EmailValidator validator, PasswordHasher hasher,
                        WelcomeEmailSender emailer, UserRepository repository) {
        _validator = validator; _hasher = hasher; _emailer = emailer; _repository = repository;
    }

    public bool Register(string email, string password) {
        if (!_validator.IsValid(email)) return false;
        _repository.Save(email, _hasher.Hash(password));
        _emailer.Send(email);
        return true;
    }
}

Anemic Domain Model

The mirror image of a God Object — instead of one class hoarding all behavior, an object is reduced to a passive bag of fields while every operation on it lives in a separate "service" that reaches into its internals from outside. Real encapsulation (module 02) means behavior lives with the data it governs, not next to it in another class.

# VIOLATION: anemic domain model — Order is a data bag; all behavior lives outside it, reaching into its fields
class OrderBad:
    def __init__(self):
        self.items: list[float] = []
        self.status = "OPEN"

class OrderServiceBad:
    def add_item(self, order: OrderBad, price: float):
        order.items.append(price)                    # reaches into Order's internals from outside

    def total(self, order: OrderBad) -> float:
        return sum(order.items)                       # logic about Order lives in a totally separate class

    def cancel(self, order: OrderBad):
        if order.status == "SHIPPED":                 # the rule "can't cancel once shipped" lives outside Order
            raise ValueError("Cannot cancel a shipped order")
        order.status = "CANCELLED"

# FIXED: behavior moves onto Order itself — the object protects its own invariants
class Order:
    def __init__(self):
        self._items: list[float] = []
        self._status = "OPEN"

    def add_item(self, price: float):
        self._items.append(price)

    def total(self) -> float:
        return sum(self._items)

    def cancel(self):
        if self._status == "SHIPPED":                 # the rule now lives with the data it governs
            raise ValueError("Cannot cancel a shipped order")
        self._status = "CANCELLED"

    @property
    def status(self) -> str:
        return self._status

// VIOLATION: anemic domain model — Order is a data bag; all behavior lives outside it, reaching into its fields
public class OrderBad {
    public List<double> Items = new List<double>();
    public string Status = "OPEN";
}

public class OrderServiceBad {
    public void AddItem(OrderBad order, double price) => order.Items.Add(price);   // reaches into Order's internals
    public double Total(OrderBad order) => order.Items.Sum();                       // logic lives outside Order

    public void Cancel(OrderBad order) {
        if (order.Status == "SHIPPED")                 // the rule "can't cancel once shipped" lives outside Order
            throw new InvalidOperationException("Cannot cancel a shipped order");
        order.Status = "CANCELLED";
    }
}

// FIXED: behavior moves onto Order itself — the object protects its own invariants
public class Order {
    private List<double> _items = new List<double>();
    public string Status { get; private set; } = "OPEN";

    public void AddItem(double price) => _items.Add(price);
    public double Total() => _items.Sum();

    public void Cancel() {
        if (Status == "SHIPPED")                        // the rule now lives with the data it governs
            throw new InvalidOperationException("Cannot cancel a shipped order");
        Status = "CANCELLED";
    }
}

Tight coupling ("train wreck" chains)

A caller that reaches through one object into another, into another, to get what it actually wants — every link in the chain is now a dependency the caller carries, even though it only cared about the last one. This is the Law of Demeter (module 05) violation in its most recognizable shape.

# VIOLATION: "train wreck" chaining — caller reaches through Customer into Wallet's internals
class WalletBad:
    def __init__(self, balance: float):
        self.balance = balance

class CustomerBad:
    def __init__(self, wallet: WalletBad):
        self.wallet = wallet

def checkout_bad(customer: CustomerBad, amount: float):
    if customer.wallet.balance < amount:                 # reaches through Customer into Wallet's internals
        raise ValueError("Insufficient funds")
    customer.wallet.balance -= amount                     # caller now depends on Wallet's internal shape too

# FIXED: Customer exposes a behavior; the caller never reaches past it (Law of Demeter, module 05)
class Wallet:
    def __init__(self, balance: float):
        self._balance = balance

    def has_at_least(self, amount: float) -> bool:
        return self._balance >= amount

    def deduct(self, amount: float):
        self._balance -= amount

class Customer:
    def __init__(self, wallet: Wallet):
        self._wallet = wallet

    def pay(self, amount: float):                          # Customer talks to its own Wallet — caller doesn't reach through
        if not self._wallet.has_at_least(amount):
            raise ValueError("Insufficient funds")
        self._wallet.deduct(amount)

def checkout(customer: Customer, amount: float):
    customer.pay(amount)                                    # one message to one direct collaborator

// VIOLATION: "train wreck" chaining — caller reaches through Customer into Wallet's internals
public class WalletBad {
    public double Balance;
    public WalletBad(double balance) { Balance = balance; }
}

public class CustomerBad {
    public WalletBad Wallet;
    public CustomerBad(WalletBad wallet) { Wallet = wallet; }
}

public class CheckoutBad {
    public void Checkout(CustomerBad customer, double amount) {
        if (customer.Wallet.Balance < amount)                 // reaches through Customer into Wallet's internals
            throw new InvalidOperationException("Insufficient funds");
        customer.Wallet.Balance -= amount;
    }
}

// FIXED: Customer exposes a behavior; the caller never reaches past it (Law of Demeter, module 05)
public class Wallet {
    private double _balance;
    public Wallet(double balance) { _balance = balance; }
    public bool HasAtLeast(double amount) => _balance >= amount;
    public void Deduct(double amount) => _balance -= amount;
}

public class Customer {
    private Wallet _wallet;
    public Customer(Wallet wallet) { _wallet = wallet; }

    public void Pay(double amount) {                          // Customer talks to its own Wallet
        if (!_wallet.HasAtLeast(amount)) throw new InvalidOperationException("Insufficient funds");
        _wallet.Deduct(amount);
    }
}

public class Checkout {
    public void Process(Customer customer, double amount) => customer.Pay(amount);   // one message to one collaborator
}

Primitive Obsession

Using raw strings, floats, and ints for things that actually carry rules — a valid email format, a currency-aware amount — instead of a small value object (module 03). The rule ends up either duplicated at every call site or missing entirely at some of them.

# VIOLATION: primitive obsession — raw str/float everywhere, no single place enforces "is this valid?"
class EmployeeBad:
    def __init__(self, email: str, salary: float, currency: str):
        self.email = email             # nothing stops an invalid email from ever being constructed
        self.salary = salary
        self.currency = currency       # nothing stops mismatched amount/currency pairs from being compared

def give_raise_bad(employee: EmployeeBad, amount: float, currency: str):
    if currency != employee.currency:                      # every call site has to remember to check this itself
        raise ValueError("Currency mismatch")
    employee.salary += amount

# FIXED: value objects (module 03) make invalid states unrepresentable, and centralize the rules
from dataclasses import dataclass

@dataclass(frozen=True)
class Email:
    value: str

    def __post_init__(self):
        if "@" not in self.value:
            raise ValueError(f"Invalid email: {self.value}")

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

    def __add__(self, other: "Money") -> "Money":
        if other.currency != self.currency:                # the rule lives in exactly one place now
            raise ValueError("Currency mismatch")
        return Money(self.amount + other.amount, self.currency)

class Employee:
    def __init__(self, email: Email, salary: Money):
        self.email = email
        self.salary = salary

    def give_raise(self, amount: Money):
        self.salary = self.salary + amount

// VIOLATION: primitive obsession — raw string/double everywhere, no single place enforces "is this valid?"
public class EmployeeBad {
    public string Email; public double Salary; public string Currency;
    public EmployeeBad(string email, double salary, string currency) {
        Email = email; Salary = salary; Currency = currency;    // nothing rejects an invalid email here
    }
}

// FIXED: value objects (module 03) make invalid states unrepresentable, and centralize the rules
public readonly struct Email {
    public readonly string Value;
    public Email(string value) {
        if (!value.Contains("@")) throw new ArgumentException($"Invalid email: {value}");
        Value = value;
    }
}

public readonly struct Money {
    public readonly double Amount; public readonly string Currency;
    public Money(double amount, string currency) { Amount = amount; Currency = currency; }

    public static Money operator +(Money a, Money b) {
        if (a.Currency != b.Currency) throw new InvalidOperationException("Currency mismatch");  // rule lives in one place
        return new Money(a.Amount + b.Amount, a.Currency);
    }
}

public class Employee {
    public Email Email; public Money Salary;
    public Employee(Email email, Money salary) { Email = email; Salary = salary; }
    public void GiveRaise(Money amount) => Salary = Salary + amount;
}

Hands-on exercises

Do each in both languages.

1. Find and fix a God Object

Take a ReportGenerator class that fetches data, calculates statistics, formats output, saves to disk, and emails the result — all in one class. Split it the same way UserManagerBad was split above.

2. Enrich an anemic model

Take an Account whose balance is only ever changed by an external AccountService.withdraw(account, amount) function that checks and mutates account.balance directly. Move withdraw onto Account itself, with the insufficient-funds check as part of the method, not a separate step the caller has to remember.

3. Untangle a train wreck

Given order.get_customer().get_address().get_city() used to print a shipping label, add a single order.shipping_city() method that returns the same value, and update the caller to use only that.

4. Replace a stringly-typed status

Take a codebase using raw string literals ("PENDING", "PAID", "SHIPPED") compared directly in if statements scattered across several classes. Replace them with an enum, and let your language's type checker/compiler find every place that needs updating.

5. Spot Shotgun Surgery

Given a hypothetical change request ("add a loyalty-points bonus to every purchase") that, in a specific poorly-organized codebase, would require editing five unrelated classes to implement, identify what poor cohesion caused that spread and consolidate the purchase-related logic into fewer, more cohesive places.

Independent challenge

No code given.

Task: Pick one classic-problem module from earlier in this track (e.g., module 13's Library, module 16's Show, or module 17's RideManager) and write a short code-smell audit: identify one plausible future feature (drawn from that module's own hands-on exercises) that, if added carelessly as more methods piled onto the same core class, would start tipping it toward a God Object or an anemic split. Then explain what's already in that module's design — an injected Strategy, an Observer, a repository-like collection — that gives you a natural seam to extend into instead of piling onto the core class.

Hint

Look specifically at Library's checkout/reserve/return trio or RideManager's request/complete/cancel trio. Ask: if fines, ratings, receipts, and notifications were each added as more methods directly on the same class rather than as new collaborator classes injected the way SplitStrategy or DriverMatchingStrategy were, at what point does the class stop having one reason to change? The seam you're looking for is usually already visible in how the module's existing dependencies were injected, not something you have to invent from scratch.

Common mistakes & troubleshooting

Treating every large class as a God Object without checking whether its size comes from one cohesive responsibility (many small, related methods) versus several unrelated ones. SRP (module
1. is about reasons to change, not line count — a class with ten methods that all serve one clear purpose isn't a God Object.
Over-correcting an anemic model by moving every operation onto the entity, including ones that legitimately span multiple objects. An Order.ship_via(carrier) that also has to call an external shipping API is orchestration, not something Order alone can own — that belongs in a service; only Order's own state changes belong on Order.
Fixing one train-wreck chain but leaving the underlying object graph exposed everywhere else. The actual fix is exposing behavior (pay()), not hiding one particular chain while every other caller still reaches straight through the same objects.
Introducing a value object for validation but forgetting to make it actually immutable. A "value object" whose fields can still be mutated after construction doesn't make invalid states unrepresentable — it just delays when they become representable.
Applying these fixes regardless of scale. A two-field, internal-only struct doesn't need full value-object ceremony with custom validation and equality; matching the fix to what the code actually needs — not more — is itself part of the judgment (YAGNI, module 05).

Checkpoint quiz

What's the actual definition of a God Object — is it about class size, or something else?
In what sense is an Anemic Domain Model the mirror image of a God Object, given they look like opposites?
What replaces a "train wreck" chain like a.getB().getC().getD(), and why does the fix reduce coupling rather than just hide it?
Why does wrapping a primitive in a value object with constructor validation prevent bugs that a scattered if check at every call site can't reliably prevent?
Name one situation where applying one of this module's fixes would be overkill.

Answers

It's about reasons to change, not size. A God Object accumulates multiple unrelated responsibilities — several independent axes along which requirements can shift, each forcing an edit to the same class — which is exactly SRP's (module 04) definition of a violation, just given a more memorable name.
Both put behavior in the wrong place relative to data. A God Object pulls too much behavior into one class; an anemic model pushes all behavior out of the class that should own it, into an external service. Neither keeps behavior located with the data it actually governs.
Exposing a behavior method on the object the caller already has (customer.pay(amount)) instead of reaching through it. This reduces coupling — not just hides it — because the caller no longer depends on Wallet's existence or shape at all; only Customer's public behavior.
Because constructor validation makes the invalid state impossible to construct in the first place — there's no code path that skips it. A scattered if check only works if every single call site remembers to include it, and any one omission lets an invalid value through undetected.
Any answer describing ceremony disproportionate to the actual risk — e.g., building a fully validated, custom-equality value object for an internal-only, two-field struct that's never exposed outside one small function and carries no real invariant to protect.

Interview questions

"Here's a class — what's wrong with it?" (handed a God Object) Walk through what responsibilities it holds, group them by "reason to change" (module 04's SRP), and propose one focused class per group, with the original class reduced to coordinating between them — the same shape as the UserManagerBad → UserManager refactor.
"This Order class has no methods, just fields, and an OrderService does everything to it — what's the issue?" It's an anemic domain model: behavior that belongs with Order's own state (like the "can't cancel a shipped order" rule) lives somewhere else entirely, so nothing actually protects Order's invariants — any code with a reference to it can mutate it into an invalid state without Order ever getting a say.
"How would you refactor a.getB().getC().getD()?" Add a method to a that returns what the caller actually needs, computed internally by walking that chain itself — the caller should send one message to one direct collaborator (Law of Demeter, module 05), not reach through three.
"Why use a value object instead of a raw string or float for something like an email address or a money amount?" A value object (module 03) centralizes validation at construction, making an invalid instance impossible to create — versus a raw primitive, where every call site that cares about validity has to remember to check it independently, and any one omission is a bug.
"How do you avoid over-applying these fixes and creating needless ceremony?" Match the fix to the actual risk: a value object earns its keep when a rule is genuinely duplicated or missed across multiple call sites; splitting a class earns its keep when it genuinely has multiple independent reasons to change. Applying either fix to code that doesn't have that problem is YAGNI (module 05) in the other direction — solving a problem the code doesn't have yet.