Practice: Diagnosing Five Classes

Five short classes, each breaking one principle; name it and the fix.

Diagnosing which principle a class breaks, from what you see in its code, is the skill practised here across all five principles. The listing holds five designs: InvoiceExporter, ReadOnlyFile, OrderReport, Invoice and BasicPrinter (with its parent Device). Each breaks exactly one SOLID principle. For each, name the principle and the fix.

class InvoiceExporter:
    def export(self, invoice, format):
        if format == "pdf":
            return f"PDF {invoice}"
        if format == "csv":
            return f"CSV {invoice}"
        raise ValueError(format)


class File:
    def __init__(self, text):
        self.text = text

    def read(self):
        return self.text

    def write(self, text):
        self.text = text


class ReadOnlyFile(File):
    def write(self, text):
        raise PermissionError("read-only")


class OrderReport:
    def __init__(self):
        self.database = PostgresConnection("reports.internal")

    def weekly_total(self):
        return sum(self.database.query("SELECT amount FROM orders"))


class Invoice:
    def total(self): ...

    def to_html(self): ...

    def save_to_disk(self, path): ...


class Device:
    def power_on(self): ...

    def print_page(self, page): ...

    def scan_page(self): ...

    def fax(self, number): ...


class BasicPrinter(Device):
    def scan_page(self):
        raise NotImplementedError("cannot scan")

    def fax(self, number):
        raise NotImplementedError("cannot fax")


class PostgresConnection:
    def __init__(self, host):
        self.host = host

    def query(self, sql):
        return [120, 80]

Five classes, five principles.

Show the answers

The principle each class breaks, and the fix
Class	Principle broken	Symptom	Fix
InvoiceExporter	Open/closed	Every new format adds a branch to `export`.	One exporter class per format behind an interface, chosen from a table.
ReadOnlyFile	Liskov substitution	`write` raises where `File.write` succeeds.	Move `write` to a `WritableFile`; `ReadOnlyFile` extends a base with `read` only.
OrderReport	Dependency inversion	Creates its own `PostgresConnection`, so a test needs a database.	Inject an `OrderSource` interface owned by the report.
Invoice	Single responsibility	Totals, HTML and disk storage change for different people.	Keep `total` on `Invoice`; move rendering and storage into their own classes.
BasicPrinter	Interface segregation	Forced to implement `scan_page` and `fax` by raising.	Split `Device` into `Printer`, `Scanner` and `Fax` interfaces.

BasicPrinter's raising methods would also break Liskov for any caller of Device. Its root cause is the size of the interface, which is why the fix is a split rather than a new parent.

← → move through sections and cards