Patterns and Review

Object-Oriented Design with UML

Course 1 · Chapter 11 · Patterns, Principles and Design Review

By now you can draw the library six ways. This chapter is about judging a design from its diagrams: recognising a known shape (a design pattern), spotting a bad one (a god class, a cycle, an anemic model), and changing a design safely, on paper first. It finishes with a real change to the library, made in the order a careful team would work: review the current design, redraw it, then change the code and check that nothing broke.

What was run, and what was not
Run here (Python 3.14, Mermaid 12.1.0): the library from chapter 9 refactored for two new requirements (the 10 old tests pass unchanged, and 13 more were added); five pattern diagrams each checked against the code by a script, which I also tampered with to prove it can fail; three smell detectors I wrote, run on the course's own code and on a deliberately flawed module; and a dependency check on a package cycle and its fix. Not re-read: the pattern catalogue (the "Gang of Four" book) is from general knowledge; patterns are explained, with their trade-offs, in Design Patterns (State is its chapter 8), and SOLID in Clean Code, SOLID & Refactoring 6 and 7. This chapter shows the diagrams, and does not re-teach the patterns. The detectors' thresholds are my own and arbitrary; they are prompts for a review, not verdicts.

Five Patterns, Drawn and Checked

A pattern is a recurring arrangement of classes that solves a recurring problem. On a diagram it is a shape: if you can recognise the shape, you know the intent without reading the code. For each pattern below there is a diagram using the library's own names, the code it describes, and a note on where the diagram and code agree.

PatternProblem it solvesIn the library
StrategyA rule that varies, to be chosen at run timeFinePolicy (how a fine is worked out), added in this chapter
ObserverTell interested parties when something happens, without the source knowing themLoanDesk publishes events to Subscribers
StateBehaviour that depends on the object's stateThe loan lifecycle of chapter 8, one class per state
FactoryOne place that decides which class to createKindFactory creating the right MemberKind
CompositeTreat one thing and a group of things the same wayHoldings: a Single copy or a Shelf of them

Strategy

chosen strategy

asks to give back

uses

LoanDesk

+return_book(barcode, today) : int

Member

+give_back(loan, copy, today, policy) : int

«interface»

FinePolicy

+fine(days_overdue, kind, item_type) : int

PerDayFine

+fine(days_overdue, kind, item_type) : int

The shape: a client that holds (or is given) an interface, and several classes that realise it. Member.give_back uses a FinePolicy without knowing which; PerDayFine is one.

"""How a fine is worked out. A Strategy: the rule can be replaced without touching Member, Loan or the desk.""" from __future__ import annotations from typing import TYPE_CHECKING, Protocol, runtime_checkable if TYPE_CHECKING: # only for the type hints: no import at run time, so no import cycle with model.py from library.domain.model import MemberKind ITEM_MULTIPLIER = {"book": 1, "dvd": 2} @runtime_checkable class FinePolicy(Protocol): def fine(self, days_overdue: int, kind: MemberKind, item_type: str) -> int: ... class PerDayFine: """Days overdue times the member kind's daily rate times the item type's multiplier (a DVD costs double).""" def fine(self, days_overdue: int, kind: MemberKind, item_type: str) -> int: return max(0, days_overdue) * kind.fine_per_day() * ITEM_MULTIPLIER[item_type]

Observer

tells

1

0..*

publishes

publishes

LoanDesk

+subscribe(subscriber)

+borrow(card, barcode, today) : Loan

+return_book(barcode, today) : int

«interface»

Subscriber

+notify(event)

ConsoleNotifier

+notify(event)

LoanRecorded

BookReturned

The shape: a subject with a list of subscribers behind an interface, and events it publishes. The desk knows Subscriber, never ConsoleNotifier.

State

current

1

1

LoanStateMachine

+send(event, today) : str

+go(new)

State

+renew(m, today)

+day_passed(m, today)

+return_book(m, today)

+declare_lost(m, today)

+close_library(m, today)

+reopen(m, today)

+enter(m)

Active

OnLoan

Overdue

Suspended

Returned

Lost

The shape: a context that points at a current state object, and one class per state. This is exactly the code of chapter 8 (loan_state_pattern.py); the diagram is checked against it. Active is the composite state from chapter 8's diagram, here as a shared parent class.

Factory and Composite

returns

creates

creates

KindFactory

+create(name) : MemberKind

«abstract»

MemberKind

+loan_limit() : int

+fine_per_day() : int

Standard

Junior

Factory: callers ask KindFactory.create("junior") and get a MemberKind. Only the factory names Standard and Junior.

children

1

0..*

«abstract»

Holdings

+count() : int

Single

+count() : int

Shelf

+add(child) : Shelf

+count() : int

Composite: a Shelf is a Holdings and holds Holdings (single copies or other shelves), so count() recurses.

"""Two small patterns the library does not otherwise need, written so every diagram in the chapter has real code behind it.""" from abc import ABC, abstractmethod from library.domain import Copy, Junior, MemberKind, Refusal, Standard class KindFactory: """Factory: one place that turns a name into the right MemberKind, so callers do not know the concrete classes.""" registry = {"standard": Standard, "junior": Junior} @classmethod def create(cls, name: str) -> MemberKind: if name not in cls.registry: raise Refusal(f"unknown member kind {name!r}") return cls.registry[name]() class Holdings(ABC): """Composite: a single copy and a whole shelf of things can both be asked how many copies they hold.""" @abstractmethod def count(self) -> int: ... class Single(Holdings): def __init__(self, copy: Copy): self.copy = copy def count(self) -> int: return 1 class Shelf(Holdings): def __init__(self, label: str): self.label = label self.children: list[Holdings] = [] def add(self, child: Holdings) -> "Shelf": self.children.append(child) return self def count(self) -> int: return sum(child.count() for child in self.children)

Are the Diagrams True?

A pattern diagram is only worth having if it matches the code. The check below reads each diagram and verifies, against the source files, that every drawn operation and attribute exists, that every drawn inheritance is real, and that every drawn realisation is real (by actually testing, at run time, that the class fits the interface):

"""Check each pattern diagram against the code it describes: operations and attributes exist, inheritance is real, realisation is real. Names only: this cannot tell whether the pattern is applied well, just that the diagram is not lying about what exists. """ import ast import importlib import pathlib import re HERE = pathlib.Path(__file__).parent CHECKS = { "strategy.mmd": ["library/domain/fines.py", "library/domain/model.py", "library/application/desk.py"], "observer.mmd": ["library/application/desk.py", "library/application/ports.py", "library/application/events.py", "library/infrastructure/notifier.py"], "state.mmd": ["loan_state_pattern.py"], "factory.mmd": ["patterns_extra.py", "library/domain/model.py"], "composite.mmd": ["patterns_extra.py"], "before.mmd": ["library_before/domain/model.py", "library_before/application/desk.py", "library_before/application/ports.py"], "after.mmd": ["library/domain/fines.py", "library/domain/model.py", "library/application/desk.py", "library/application/ports.py", "library/infrastructure/notifier.py"], } MODULES = ["library.domain", "library.application", "library.infrastructure", "patterns_extra", "loan_state_pattern"] def diagram(path: pathlib.Path): ops, attrs, inherits, realises, current = {}, {}, [], [], None for line in path.read_text(encoding="utf8").splitlines(): c = re.match(r"\s*class (\w+)", line) if c: current = c.group(1) ops.setdefault(current, set()) attrs.setdefault(current, set()) if line.rstrip().endswith("}") is False and "{" not in line: current = None if False else current continue o = re.match(r"\s*\+(\w+)\(.*\)", line) if o and current: ops[current].add(o.group(1)) continue a = re.match(r"\s*\+(\w+):", line) if a and current: attrs[current].add(a.group(1)) continue i = re.match(r"\s*(\w+) --\|> (\w+)", line) if i: inherits.append((i.group(1), i.group(2))) r = re.match(r"\s*(\w+) \.\.\|> (\w+)", line) if r: realises.append((r.group(1), r.group(2))) return ops, attrs, inherits, realises def code(paths): methods, fields, bases = {}, {}, {} for rel in paths: for node in ast.parse((HERE / rel).read_text(encoding="utf8")).body: if isinstance(node, ast.ClassDef): methods.setdefault(node.name, set()).update(f.name for f in node.body if isinstance(f, ast.FunctionDef)) fields.setdefault(node.name, set()).update(s.target.id for s in node.body if isinstance(s, ast.AnnAssign) and isinstance(s.target, ast.Name)) bases[node.name] = {b.id for b in node.bases if isinstance(b, ast.Name)} return methods, fields, bases def find_class(name: str): for module in MODULES: mod = importlib.import_module(module) if hasattr(mod, name): return getattr(mod, name) try: # classes not exported by a package's __init__ for dotted in ("library.domain.model", "library.domain.fines", "library.application.ports", "library.application.events"): mod = importlib.import_module(dotted) if hasattr(mod, name): return getattr(mod, name) except ImportError: pass return None def main() -> int: problems = 0 for name, files in CHECKS.items(): ops, attrs, inherits, realises = diagram(HERE / "diagrams" / name) methods, fields, bases = code(files) issues = [] for cls, wanted in ops.items(): if cls in methods: issues += [f"{cls}.{op}() is drawn but not in the code" for op in sorted(wanted - methods[cls])] for cls, wanted in attrs.items(): if wanted and cls in fields: issues += [f"{cls}.{a} is drawn but is not a field in the code" for a in sorted(wanted - fields[cls])] for sub, base in inherits: if base not in bases.get(sub, set()): issues.append(f"{sub} --|> {base} is drawn, but {sub} does not inherit from {base} in the code") for impl, iface in realises: a, b = find_class(impl), find_class(iface) try: ok = a is not None and b is not None and issubclass(a, b) except TypeError: ok = False if not ok: issues.append(f"{impl} ..|> {iface} is drawn, but {impl} does not fit {iface} in the code") print(f"{name:<14} {'OK' if not issues else str(len(issues)) + ' problem(s)'}") for i in issues: print(" ", i) problems += len(issues) return 1 if problems else 0 if __name__ == "__main__": raise SystemExit(main())
strategy.mmd OK observer.mmd OK state.mmd OK factory.mmd OK composite.mmd OK before.mmd OK after.mmd OK

All seven diagrams (the five patterns plus the library's before and after, below) pass. A check that passes first time deserves suspicion, so here it is on three deliberately tampered diagrams: an operation renamed in the strategy diagram, a wrong interface in the observer diagram, and a wrong parent in the state diagram:

strategy.mmd 1 problem(s) FinePolicy.calculate() is drawn but not in the code observer.mmd 1 problem(s) ConsoleNotifier ..|> FinePolicy is drawn, but ConsoleNotifier does not fit FinePolicy in the code state.mmd 1 problem(s) OnLoan --|> State is drawn, but OnLoan does not inherit from State in the code (exit status 1)

Each lie is caught and named. What the check cannot say is whether the pattern is used well: it checks names, as in chapter 10.

Principles You Can See on a Diagram

SOLID is covered properly in Clean Code, SOLID & Refactoring 6 and 7. What this course adds is that several of its ideas show up as shapes:

  • Dependency inversion: interfaces are owned by the client, and arrows point at them. In the after diagram below, LoanDesk has three arrows, all to interfaces (Catalogue, Subscriber, FinePolicy), and every concrete class has a dotted arrow into an interface. A concrete class with an arrow pointing out of the desk would be a sign that high-level code depends on low-level detail. Chapter 9's dependency check makes the package-level version of this a test.
  • Open for extension, closed for change: a new box that touches no old box. Adding a subscriber is a new class; nothing existing is edited (Exercise 2 proves it by fingerprinting the library's files before and after).
  • Cohesion: clusters. Classes that change together should be close together and linked; classes that do not should not be. In chapter 9's package diagram, the clusters are the packages. Inside a class, the equivalent is "do its methods use the same data?", which the god class detector below measures.
  • Coupling: count the arrows leaving a box. A box with arrows to everything has too many reasons to change. A box with none (Copy, in the domain) is easy to understand and test.

A Design Review Checklist

A review is a short, structured look at a design (diagrams plus a few key files) by someone other than its author, done before the cost of change is high. Questions, each with what to look for on a diagram and, where one exists, an automatic check from this course:

QuestionLook for on the diagramAutomatic check
Does each class have one job?A box with many unrelated operations; a name ending in Manager, Handler or UtilGod class detector (below)
Do the dependencies point the right way?A high-level box with arrows to concrete low-level boxesPackage rules (chapter 9)
Are there cycles?Arrows that return to where they startedPackage cycle check (chapter 9); mutual references (below)
Where does the behaviour live?Boxes with fields and no operations, beside one box with all the operations and all the arrowsAnemic detector (below); reach-in count (chapter 6)
What varies, and is it isolated?if kind == ... chains; one concrete class per variation with no interfaceSearch for type-tag comparisons (chapter 6)
What will the next likely change touch?Trace the change across the boxes: how many must be edited?Diff statistics after a trial change (this chapter)
What if something fails or runs twice?Calls to outside things with no failure path (the e-mail server, a second desk)Tests for failure paths (the broken subscriber below)
Do the diagrams still match the code?Operation or class names that do not existDiagram check (above), drift check (chapter 10)

Finding a God Class, a Cycle and an Anemic Model

Three common problems, and their shape on a diagram:

  • God class: one big box with arrows out to nearly everything, and a long list of operations that fall into several unrelated groups (lending, e-mail, reports, files, search).
  • Cyclic dependency: a loop of arrows, typically between packages, so that neither side can be understood or tested alone (chapter 9 showed one stop the program importing).
  • Anemic model: chapter 6's picture: several boxes with fields and no operations, and one service box with every operation and an arrow to each of them.

Detectors can point at candidates. These are mine, small, and honest about being crude (they measure: for a god class, how many unrelated groups a class's methods fall into, linking two methods when they use the same field or call each other, a measure called LCOM4 as far as I remember; for mutual references, which classes name each other; for anemia, how many classes hold only data and how often a service reaches into them from outside):

"""Three design smells, found by counting. My own simple detectors, written for this chapter; thresholds are arbitrary and say so. god class a class whose methods split into unrelated groups (LCOM4 > 1: methods are linked when they use a common field or call each other; each separate group is a separate job), together with a large number of methods mutual refs two classes that each name the other (not always wrong: a two-way association is legitimate, but it is worth a look) anemic classes with several fields and no behaviour, beside a class that reads and writes their fields from outside py smells.py module.py [module.py ...] """ import ast import pathlib import sys GOD_METHODS = 8 # arbitrary: this many methods in one class is "a lot" for this course's examples def classes_of(path: str): tree = ast.parse(pathlib.Path(path).read_text(encoding="utf8")) return {n.name: n for n in tree.body if isinstance(n, ast.ClassDef)} def fields_of(cls: ast.ClassDef) -> set[str]: out = {s.target.id for s in cls.body if isinstance(s, ast.AnnAssign) and isinstance(s.target, ast.Name)} for fn in cls.body: if isinstance(fn, ast.FunctionDef) and fn.name == "__init__": out |= {n.attr for n in ast.walk(fn) if isinstance(n, ast.Attribute) and isinstance(n.value, ast.Name) and n.value.id == "self" and isinstance(n.ctx, ast.Store)} return out def lcom4(cls: ast.ClassDef) -> tuple[int, list[list[str]]]: """Connected groups of methods, linking two methods when they touch the same field or one calls the other.""" own = fields_of(cls) methods = {f.name: f for f in cls.body if isinstance(f, ast.FunctionDef) and not f.name.startswith("__")} uses, calls = {}, {} for name, fn in methods.items(): uses[name] = {n.attr for n in ast.walk(fn) if isinstance(n, ast.Attribute) and isinstance(n.value, ast.Name) and n.value.id == "self" and n.attr in own} calls[name] = {n.func.attr for n in ast.walk(fn) if isinstance(n, ast.Call) and isinstance(n.func, ast.Attribute) and isinstance(n.func.value, ast.Name) and n.func.value.id == "self" and n.func.attr in methods} parent = {m: m for m in methods} def find(x): while parent[x] != x: parent[x] = parent[parent[x]] x = parent[x] return x names = list(methods) for i, a in enumerate(names): for b in names[i + 1:]: if uses[a] & uses[b] or b in calls[a] or a in calls[b]: parent[find(a)] = find(b) groups: dict[str, list[str]] = {} for m in names: groups.setdefault(find(m), []).append(m) return len(groups), list(groups.values()) def named_classes(cls: ast.ClassDef, all_names: set[str]) -> set[str]: found = set() for n in ast.walk(cls): if isinstance(n, ast.Name) and n.id in all_names and n.id != cls.name: found.add(n.id) if isinstance(n, ast.Constant) and isinstance(n.value, str): # forward references written as strings for name in all_names: if name != cls.name and name in n.value.replace("|", " ").split(): found.add(name) return found def analyse(path: str) -> list[str]: classes = classes_of(path) report = [] for name, node in classes.items(): methods = [f for f in node.body if isinstance(f, ast.FunctionDef) and not f.name.startswith("__")] count, groups = lcom4(node) if count > 1 and len(methods) >= GOD_METHODS: report.append(f"GOD CLASS {name}: {len(methods)} methods in {count} unrelated groups: " + "; ".join("{" + ", ".join(g) + "}" for g in groups)) refs = {n: named_classes(c, set(classes)) for n, c in classes.items()} seen = set() for a in refs: for b in refs[a]: if a in refs.get(b, set()) and (b, a) not in seen: seen.add((a, b)) report.append(f"MUTUAL {a} and {b} each name the other") data_only = [n for n, c in classes.items() if len(fields_of(c)) >= 2 and not [f for f in c.body if isinstance(f, ast.FunctionDef) and not f.name.startswith("__")]] if data_only: biggest = max(classes, key=lambda n: len([f for f in classes[n].body if isinstance(f, ast.FunctionDef)])) field_names = {f for n in data_only for f in fields_of(classes[n])} poking = sum(1 for n in ast.walk(classes[biggest]) if isinstance(n, ast.Attribute) and not (isinstance(n.value, ast.Name) and n.value.id == "self") and n.attr in field_names) report.append(f"ANEMIC? {len(data_only)} of {len(classes)} classes hold only data ({', '.join(data_only)}); " f"{biggest} reads or writes their fields {poking} times from outside") return report if __name__ == "__main__": for path in sys.argv[1:]: print(path) for line in analyse(path) or ["(nothing flagged)"]: print(" " + line)

The flawed module they are run on (a manager that does everything, two classes that reference each other, two data-only classes):

"""A deliberately flawed library module, for the design review exercises: a god class, mutual references, and data-only classes.""" from dataclasses import dataclass, field from datetime import date, timedelta @dataclass class Copy: barcode: str on_loan: bool = False @dataclass class Fine: card: str pence: int paid: bool = False @dataclass class Member: card: str email: str loans: list = field(default_factory=list) def record_loan(self, loan: "Loan") -> None: self.loans.append(loan) loan.member = self # the Member reaches into the Loan ... @dataclass class Loan: barcode: str due_on: date member: "Member | None" = None def overdue_fine(self, today: date) -> int: owner: Member = self.member # ... and the Loan reaches back into the Member return max(0, (today - self.due_on).days) * (5 if owner.card.startswith("J") else 10) class LibraryManager: """Does everything: lending, fines, e-mail, reports, files, and searching.""" def __init__(self, smtp_host: str, data_file: str): self.members: dict[str, Member] = {} self.copies: dict[str, Copy] = {} self.fines: list[Fine] = [] self.smtp_host = smtp_host self.sent: list[str] = [] self.data_file = data_file self.report_lines: list[str] = [] self.catalogue_index: dict[str, list[str]] = {} # lending def borrow(self, card: str, barcode: str, today: date) -> Loan: member, copy = self.members[card], self.copies[barcode] if copy.on_loan: raise ValueError("already on loan") loan = Loan(barcode, today + timedelta(days=21)) member.record_loan(loan) copy.on_loan = True return loan def return_book(self, card: str, barcode: str, today: date) -> int: member = self.members[card] loan = next(l for l in member.loans if l.barcode == barcode) fine = loan.overdue_fine(today) if fine: self.fines.append(Fine(card, fine)) self.send_email(member.email, f"You owe {fine}p") self.copies[barcode].on_loan = False member.loans.remove(loan) return fine # e-mail def send_email(self, to: str, text: str) -> None: self.sent.append(f"{self.smtp_host}: to {to}: {text}") # reporting def build_report(self) -> list[str]: self.report_lines = [f"{f.card}: {f.pence}p" for f in self.fines if not f.paid] return self.report_lines def print_report(self) -> str: return "\n".join(self.report_lines) # persistence def save(self) -> str: return f"saved {len(self.members)} members to {self.data_file}" def load(self) -> str: return f"loaded from {self.data_file}" # searching def index_title(self, title: str, barcode: str) -> None: self.catalogue_index.setdefault(title.lower(), []).append(barcode) def search(self, word: str) -> list[str]: return self.catalogue_index.get(word.lower(), [])

The results on the course's own earlier code and the flawed module:

library_a.py ANEMIC? 3 of 5 classes hold only data (Member, Copy, Loan); LibraryService reads or writes their fields 22 times from outside library_b.py (nothing flagged) flawed_library.py GOD CLASS LibraryManager: 9 methods in 2 unrelated groups: {borrow, return_book, send_email, build_report, print_report, save, load}; {index_title, search} MUTUAL Member and Loan each name the other ANEMIC? 2 of 5 classes hold only data (Copy, Fine); LibraryManager reads or writes their fields 7 times from outside
  • Chapter 6's version A is flagged as anemic (3 of its 5 classes hold only data and the service reaches into them 22 times), and version B is not flagged. The detector reproduces the judgement of chapter 6 from the code alone.
  • The flawed module gets all three flags: the god class, the mutual reference between Member and Loan, and an anemic suspicion for Copy and Fine.
  • The god-class verdict is weaker than it looks. A reader sees five jobs in LibraryManager (lending, e-mail, reporting, persistence, searching). The detector found only two groups, because the jobs share fields (the member list, the fines) and one method calls another. Exercise 1 shows this. A detector's "one group" does not clear a class, and its "two" is a floor, not a count.
  • The mutual reference is not necessarily wrong. A member and its loans may legitimately know each other. Here the flaw is how: each reaches into the other's data (loan.member = self; owner.card.startswith("J") to choose a fine rate: a rule hiding in the wrong class, with a type test in disguise). The detector asks the question; the review answers it.

The same code scanned file by file in the current library finds nothing to flag:

library/domain/model.py (nothing flagged) library/domain/fines.py (nothing flagged) library/application/desk.py (nothing flagged) library/infrastructure/jsonfile.py (nothing flagged) library/infrastructure/memory.py (nothing flagged)

And a package-level cycle, on a tiny tree where billing and members import each other, caught by chapter 9's checker:

analysed 3 files, found 2 cross-package imports package dependencies found in the code (from -> to : first place): billing -> members library\billing\__init__.py:1 (library.members) members -> billing library\members\__init__.py:1 (library.billing) R1 direction: R2 no cycles: VIOLATION cycle: billing -> members -> billing R3 public API only: result: 1 violation(s)

Refactoring the Library: Diagram First, Then Code

Two requirements arrive. One: a DVD that comes back late should cost double a book. Two: the member should be sent a notice whenever a loan is recorded and when a late return adds a fine. This is the chapter-9 library:

1

0..*

fine uses

LoanDesk

+borrow(card, barcode, today) : Loan

+return_book(barcode, today) : int

«interface»

Catalogue

+member(card) : Member

+copy(barcode) : Copy

+open_loan_for(barcode)

+save()

Member

+borrow(copy, today) : Loan

+give_back(loan, copy, today) : int

Loan

+fine(today, kind) : int

«abstract»

MemberKind

+loan_limit() : int

+fine_per_day() : int

The library's core before the change (checked against the code: all drawn operations exist).

The Review

Applying the checklist's "what will the next change touch?" question to requirement one: the fine is worked out in Loan.fine(today, kind), which knows only the member's kind. A copy's type (book or DVD) lives nowhere, and a loan holds its copy as a barcode, not as an object. To vary by item type I would have to edit Loan, Member.give_back and Copy and keep them consistent, and the next rule would edit all three again. That is a fine rule with no home of its own. For requirement two: the obvious place to send a notice is inside LoanDesk.borrow. That would make the desk (high-level code) depend on e-mail or printing (low-level detail), would make every desk test send messages, and would let a broken mail server stop a loan being recorded. It would also be the first step towards the god class above.

The Redrawn Design

Before touching code, the new shape, as a diagram: the fine rule becomes a Strategy (FinePolicy, with the item type on Copy passed to it), and the notices become an Observer arrangement (Subscriber and ConsoleNotifier). Both interfaces are owned by the code that uses them.

1

0..*

1

0..*

uses

reads item_type

LoanDesk

+subscribe(subscriber)

+borrow(card, barcode, today) : Loan

+return_book(barcode, today) : int

«interface»

Catalogue

+member(card) : Member

+copy(barcode) : Copy

+open_loan_for(barcode)

+save()

«interface»

Subscriber

+notify(event)

ConsoleNotifier

+notify(event)

«interface»

FinePolicy

+fine(days_overdue, kind, item_type) : int

PerDayFine

+fine(days_overdue, kind, item_type) : int

Member

+borrow(copy, today) : Loan

+give_back(loan, copy, today, policy) : int

Loan

+days_overdue(today) : int

Copy

+item_type: str

«abstract»

MemberKind

+loan_limit() : int

+fine_per_day() : int

After. LoanDesk depends on three interfaces; each new class points into one of them. (This diagram is also checked against the code: see the seven-diagram check above.)

The Code

Now the change. The unchanged parts of the package are the safety net: chapter 9's 10 tests, run before and after without being edited. The changes:

"""Things that happen at the desk, announced to whoever has subscribed (an Observer arrangement).""" from dataclasses import dataclass from datetime import date @dataclass(frozen=True) class LoanRecorded: card: str barcode: str due_on: date @dataclass(frozen=True) class BookReturned: card: str barcode: str fine_pence: int
"""The interfaces the application REQUIRES. Defined here, in the application, not beside the code that provides them.""" from typing import Protocol, runtime_checkable from library.domain import Copy, Loan, Member @runtime_checkable class Catalogue(Protocol): def member(self, card: str) -> Member: ... def copy(self, barcode: str) -> Copy: ... def open_loan_for(self, barcode: str) -> tuple[Member, Loan]: ... def save(self) -> None: ... @runtime_checkable class Subscriber(Protocol): """Anything that wants to hear about desk events (an email sender, a log, a counter).""" def notify(self, event: object) -> None: ...
"""The use cases of the lending desk. Depends on the domain and on the interfaces it requires; knows nothing about storage or about who is listening.""" from datetime import date from library.application.events import BookReturned, LoanRecorded from library.application.ports import Catalogue, Subscriber from library.domain import FinePolicy, Loan class LoanDesk: def __init__(self, catalogue: Catalogue, subscribers: tuple[Subscriber, ...] = (), fine_policy: FinePolicy | None = None): self.catalogue = catalogue self.subscribers = list(subscribers) self.fine_policy = fine_policy self.errors: list[str] = [] # a subscriber that failed: recorded, never allowed to undo a loan def subscribe(self, subscriber: Subscriber) -> None: self.subscribers.append(subscriber) def borrow(self, card: str, barcode: str, today: date) -> Loan: loan = self.catalogue.member(card).borrow(self.catalogue.copy(barcode), today) self.catalogue.save() self._publish(LoanRecorded(card, barcode, loan.due_on)) return loan def return_book(self, barcode: str, today: date) -> int: member, loan = self.catalogue.open_loan_for(barcode) fine = member.give_back(loan, self.catalogue.copy(barcode), today, self.fine_policy) self.catalogue.save() self._publish(BookReturned(member.card, barcode, fine)) return fine def _publish(self, event: object) -> None: for subscriber in self.subscribers: try: subscriber.notify(event) except Exception as e: # noqa: BLE001 - one broken subscriber must not break the desk or silence the others self.errors.append(f"{type(subscriber).__name__} failed on {type(event).__name__}: {type(e).__name__}")
"""A subscriber that turns desk events into messages (printed here; an email sender would be another subscriber).""" from library.application import BookReturned, LoanRecorded class ConsoleNotifier: def __init__(self, write=print): self.write = write def notify(self, event: object) -> None: if isinstance(event, LoanRecorded): self.write(f"notice to {event.card}: {event.barcode} is due back on {event.due_on}") elif isinstance(event, BookReturned) and event.fine_pence: self.write(f"notice to {event.card}: a fine of {event.fine_pence}p was added for {event.barcode}")

Three decisions in the desk are worth reading twice. A subscriber's failure never undoes a loan: the loan is saved first, then published, and each notification is isolated, so a broken subscriber is recorded and the others still hear. The desk publishes after saving, so subscribers never hear about a loan that was not recorded. And the old data files still load: files without an item type load as books (item_type defaults to "book").

What the Change Touched

file status lines added lines removed application/__init__.py changed 3 2 application/desk.py changed 21 5 application/events.py NEW 17 0 application/ports.py changed 8 1 domain/__init__.py changed 2 1 domain/fines.py NEW 21 0 domain/model.py changed 7 4 infrastructure/__init__.py changed 3 2 infrastructure/jsonfile.py changed 3 2 infrastructure/notifier.py NEW 13 0 ui/cli.py changed 2 2 TOTAL 100 19

Three new files and eight edited, 100 lines added and 19 removed. The domain's model.py, where the fine rule used to live, changed by only seven lines added and four removed. Not one line of the member, loan or catalogue behaviour changed except the fine's calculation moving out. Whether that is "a small change" is a judgement; the point is that it is countable, and that the same question can be asked of the next change.

Does It Still Work, and Is the Architecture Intact?

"""Tests for the refactored library: the strategy, the subscribers, and the promise that old data still loads.""" import json import os import tempfile import unittest from datetime import date from library.application import BookReturned, LoanDesk, LoanRecorded, Subscriber from library.domain import Copy, FinePolicy, Member, PerDayFine from library.infrastructure import ConsoleNotifier, InMemoryCatalogue, JsonCatalogue TODAY = date(2026, 10, 10) LATE = date(2026, 11, 3) # a loan taken on TODAY falls due on 2026-10-31: this is 3 days late def catalogue(): return InMemoryCatalogue([Member("M1", date(2027, 1, 1), "standard")], [Copy("B1"), Copy("D1", item_type="dvd")]) class Recorder: def __init__(self): self.events = [] def notify(self, event): self.events.append(event) class Broken: def notify(self, event): raise RuntimeError("the mail server is down") class Strategy(unittest.TestCase): def test_a_dvd_costs_double_for_the_same_lateness(self): desk = LoanDesk(catalogue()) desk.borrow("M1", "B1", TODAY) desk.borrow("M1", "D1", TODAY) self.assertEqual(desk.return_book("B1", LATE), 30) # M1 now has a fine, but returning is still allowed: only borrowing is blocked self.assertEqual(desk.return_book("D1", LATE), 60) def test_the_default_policy_satisfies_the_interface(self): self.assertIsInstance(PerDayFine(), FinePolicy) def test_a_different_policy_can_be_plugged_in_without_changing_anything_else(self): class Amnesty: def fine(self, days_overdue, kind, item_type): return 0 desk = LoanDesk(catalogue(), fine_policy=Amnesty()) desk.borrow("M1", "B1", TODAY) self.assertEqual(desk.return_book("B1", LATE), 0) class Observer(unittest.TestCase): def test_subscribers_hear_events_in_order(self): rec = Recorder() desk = LoanDesk(catalogue(), subscribers=(rec,)) desk.borrow("M1", "B1", TODAY) desk.return_book("B1", LATE) self.assertEqual(rec.events, [LoanRecorded("M1", "B1", date(2026, 10, 31)), BookReturned("M1", "B1", 30)]) def test_a_broken_subscriber_neither_undoes_the_loan_nor_silences_the_others(self): rec = Recorder() desk = LoanDesk(catalogue(), subscribers=(Broken(), rec)) loan = desk.borrow("M1", "B1", TODAY) self.assertEqual(loan.barcode, "B1") # the loan stands self.assertEqual(len(rec.events), 1) # the next subscriber still heard it self.assertEqual(desk.errors, ["Broken failed on LoanRecorded: RuntimeError"]) def test_the_desk_does_not_know_who_is_listening(self): self.assertIsInstance(Recorder(), Subscriber) self.assertIsInstance(ConsoleNotifier(), Subscriber) def test_console_notifier_messages(self): lines = [] desk = LoanDesk(catalogue(), subscribers=(ConsoleNotifier(lines.append),)) desk.borrow("M1", "B1", TODAY) desk.return_book("B1", date(2026, 10, 31)) # on time: no fine message self.assertEqual(lines, ["notice to M1: B1 is due back on 2026-10-31"]) class Compatibility(unittest.TestCase): def test_a_data_file_from_before_item_types_still_loads_as_books(self): old = {"members": [{"card": "M1", "expires_on": "2027-01-01", "kind": "standard", "fines_pence": 0, "loans": []}], "copies": [{"barcode": "B1", "on_loan": False}]} with tempfile.TemporaryDirectory() as d: path = os.path.join(d, "old.json") with open(path, "w", encoding="utf8") as f: json.dump(old, f) self.assertEqual(JsonCatalogue(path).copy("B1").item_type, "book") def test_item_type_survives_a_save_and_load(self): with tempfile.TemporaryDirectory() as d: path = os.path.join(d, "new.json") c = JsonCatalogue(path) c.add(copy=Copy("D1", item_type="dvd")) c.save() self.assertEqual(JsonCatalogue(path).copy("D1").item_type, "dvd") if __name__ == "__main__": unittest.main()
Ran 23 tests in 0.223s OK

All 23 tests pass: chapter 9's 10 (unmodified), the 9 new behaviour tests, the 2 architecture tests, and 2 for the extra patterns. The dependency rules from chapter 9 are unchanged and still clean (the new infrastructure -> application edge is the notifier importing the events, which the rules allow). And running the command-line program shows the observer at work:

$ py -m library.ui.cli --data library.json init initialised: library.json $ ... borrow M1 B1 --today 2026-10-10 notice to M1: B1 is due back on 2026-10-31 loan recorded: B1 due 2026-10-31 $ ... return B1 --today 2026-11-03 notice to M1: a fine of 30p was added for B1 returned; fine 30p

Usual Mistakes

MistakeFix
Reaching for a pattern before there is a problemWait for the second variation or the second listener; the diagram of the simple version is better than a pattern with one member
Naming a class after its pattern ("StrategyManager")Name it for its job (FinePolicy); the diagram shows the shape
Treating a smell detector's silence as a clean bill of healthIt gave two groups for five jobs; read the class
A pattern diagram that is wishfulCheck it against the code, and tamper with the check once to see it fail
Refactoring with no safety netRun the old tests before and after, unedited; add new ones for the new behaviour
Observers that can break the subjectIsolate each notification and record failures
Reviewing the code but not the changeCount what a plausible next change would touch
Mistakes while preparing this chapter
  • My first Strategy created an import cycle. The default policy lived in fines.py, which needed MemberKind from model.py, while model.py needed PerDayFine. I first hid the cycle with an import inside a method and a comment explaining why: a warning sign I had written down myself. The proper fix is for fines.py to import MemberKind only for type hints (under TYPE_CHECKING), so nothing is imported at run time and there is no cycle. Takeaway: a comment that explains why an import is in the wrong place is a design problem, not a solution.
  • A scripted edit put a comment inside an expression. Patching jsonfile.py with a text-replacement script inserted my trailing comment before a closing parenthesis, and the file stopped parsing (SyntaxError: '(' was never closed). Running the tests straight away found it in seconds. This is the same kind of slip as in chapters 7 and 10: use the editor's replace for code edits, and run the code immediately.
  • The god class detector under-counted. I wrote the flawed module with five jobs and expected the detector to find five. It found two. I kept the result in the chapter and added Exercise 1 on it. Takeaway: validate a detector on a case where you already know the answer, and report the gap.
  • An over-strict guard in my own tool, found by a new exercise. Chapter 9's dependency checker reported "nothing was analysed" when a fixed tree had no imports between packages at all, which is a legitimate state. Exercise 3 hit it. I changed the guard to fail only when no files are found, corrected chapter 9's script and its text, and re-ran its tests. Takeaway: a guard against "checking nothing" must not mistake "nothing to report" for "nothing checked".
  • A check that passed first time. The diagram check said every diagram was fine on its first run. That is the moment to distrust it, so I wrote the tampered-diagram demonstration. It is in the chapter because the first pass alone proves nothing.

Hands-On Exercises

Exercise 1

Run the smell detector on the flawed module, then list the jobs you see in LibraryManager yourself and compare with what the detector reports. Explain the difference, and say how you would split the class (name the new classes and the interfaces between them).

📄 View solution
Exercise 2

Add a third subscriber to the refactored desk: a counter of how many loans fall due on each day. Write it without editing the library package, run it alongside the console notifier, and prove the library's files did not change.

📄 View solution
Exercise 3

Build a two-package tree where billing and members import each other and show the dependency checker reporting the cycle. Remove the cycle by having members declare the interface it needs and billing provide it, show the checker clean, and show the fixed code running.

📄 View solution

Chapter 11 Quick Reference

  • Patterns are shapes on a diagram: Strategy (client + interface + implementations), Observer (subject + subscriber interface), State (context + one class per state), Factory (one creator), Composite (a node that contains nodes); this chapter shows the diagrams, Design Patterns teaches the patterns
  • Check pattern diagrams against the code (operations, inheritance, realisation), and tamper with the check once to see it fail
  • Principles as shapes: interfaces owned by the client with arrows pointing at them; new behaviour as a new box that touches no old box; clusters for cohesion; few arrows out of a box
  • Review questions: one job per class? dependencies pointing the right way? cycles? where does behaviour live? what varies? what will the next change touch? what if it fails? do the diagrams match the code?
  • Smell detectors point, people decide: god class (methods in unrelated groups, a floor on the count), mutual references (legitimate or not), anemic (data-only classes beside a service that reaches in)
  • Refactor on the diagram first, change the code with the old tests as the safety net, then count what the change touched and check the architecture rules
  • An observer must not be able to break the subject: save first, publish after, isolate and record failures
Coming next
Object-Oriented Design with UML 12 (Capstone): design a new system, a workshop-booking service, from requirements to use cases, domain model, class design, key sequence and state diagrams and a component view, all as diagram-as-code files, implement and run the core in Python, and produce a short design document. Course complete, with pointers to Design Patterns, Software Architecture Fundamentals and Clean Code, SOLID & Refactoring.