Bad Structure Anti-Patterns — Exercises¶

Category: Development Anti-Patterns → Bad Structure — hands-on practice fixing the shapes that resist change. Covers (collectively): God Object · Spaghetti Code · Lava Flow · Boat Anchor · Arrow Anti-Pattern

These are fix-it exercises, not recognition quizzes. For each one you are given a problem statement, starting code (in Go, Java, or Python — the language varies on purpose), acceptance criteria, and a collapsible solution. The point is to make the change: split a God Object, flatten an arrow, prove code dead and delete it, and so on.

How to use this file. Read the problem, try it in your editor before opening the solution, then compare. The "why it's better" note under each solution matters more than the diff — structure is a means, not an end. Refer back to junior.md for the shapes and middle.md for the countermoves.

Table of Contents¶

Exercise 1 — Flatten the gatekeeper¶

Anti-pattern: Arrow · Language: Go · Difficulty: ★ easy

The success path is buried five indents deep. Flatten it.

// withdraw returns the new balance, or an error explaining why it failed.
func withdraw(acc *Account, amount int) (int, error) {
    if acc != nil {
        if !acc.Frozen {
            if amount > 0 {
                if acc.Balance >= amount {
                    acc.Balance -= amount
                    return acc.Balance, nil
                } else {
                    return 0, errors.New("insufficient funds")
                }
            } else {
                return 0, errors.New("amount must be positive")
            }
        } else {
            return 0, errors.New("account frozen")
        }
    }
    return 0, errors.New("nil account")
}

Acceptance criteria - The happy path (acc.Balance -= amount; return ...) is at the lowest indentation level and is the last statement. - Behavior is identical: same errors for the same inputs, in the same precedence. - No else blocks remain.

Hint: invert each condition into a guard clause that returns early. Preserve the original order of checks so error precedence does not change.

func withdraw(acc *Account, amount int) (int, error) {
    if acc == nil {
        return 0, errors.New("nil account")
    }
    if acc.Frozen {
        return 0, errors.New("account frozen")
    }
    if amount <= 0 {
        return 0, errors.New("amount must be positive")
    }
    if acc.Balance < amount {
        return 0, errors.New("insufficient funds")
    }
    acc.Balance -= amount
    return acc.Balance, nil
}

Exercise 2 — Kill the boolean flags¶

Anti-pattern: Spaghetti (flag arguments) · Language: Java · Difficulty: ★ easy

One method does four different jobs depending on its flags. Callers can pass nonsense combinations.

// Three booleans = up to eight behaviors crammed into one body.
void notify(User user, String text, boolean sms, boolean push, boolean urgent) {
    if (sms) {
        if (urgent) smsGateway.sendPriority(user.phone(), text);
        else        smsGateway.send(user.phone(), text);
    }
    if (push) {
        if (urgent) pushService.sendHighPriority(user.deviceToken(), text);
        else        pushService.send(user.deviceToken(), text);
    }
    // What does notify(u, "hi", false, false, true) even do? Nothing. Silently.
}

Acceptance criteria - A caller cannot express a no-op or contradictory combination. - Each delivery channel is reachable through a clearly named method. - Urgency is modeled so that "urgent" is a property of the request, not a fourth multiplier.

Hint: the channel selection (sms/push) is dispatch; the urgency is a property. Split the channels into methods and pass priority as a typed value, not a boolean.

enum Priority { NORMAL, URGENT }

void notifyBySms(User user, String text, Priority p) {
    if (p == Priority.URGENT) smsGateway.sendPriority(user.phone(), text);
    else                      smsGateway.send(user.phone(), text);
}

void notifyByPush(User user, String text, Priority p) {
    if (p == Priority.URGENT) pushService.sendHighPriority(user.deviceToken(), text);
    else                      pushService.send(user.deviceToken(), text);
}

Exercise 3 — Prove the branch is dead¶

Anti-pattern: Lava Flow · Language: Python · Difficulty: ★ easy

Here is a function with a branch nobody trusts. Your job is not to guess — it is to prove the branch is dead, then delete it.

def price_for(plan: str, seats: int) -> int:
    if plan == "free":
        return 0
    elif plan == "team":
        return 12 * seats
    elif plan == "enterprise":
        return 20 * seats
    elif plan == "legacy_v1":
        # TODO: still needed? broke billing in 2021 when someone tried to remove it
        return 8 * seats
    raise ValueError(f"unknown plan {plan}")

You have access to the codebase and a year of production logs.

Acceptance criteria - List the evidence you would gather, not just "I think it's unused." - Only after the evidence supports it, produce the deleted version.

Hint: combine static evidence (call sites + git history) with runtime evidence (does the branch fire?).

git grep -n '"legacy_v1"'      # who ever passes this plan string?

SELECT plan, COUNT(*) FROM subscriptions GROUP BY plan;
-- legacy_v1: 0 rows  →  no live customer is on this plan

elif plan == "legacy_v1":
    log.warning("legacy_v1 pricing hit", extra={"seats": seats})
    return 8 * seats

def price_for(plan: str, seats: int) -> int:
    prices = {"free": 0, "team": 12, "enterprise": 20}
    if plan not in prices:
        raise ValueError(f"unknown plan {plan}")
    return prices[plan] * seats

remove dead legacy_v1 pricing branch

0 subscriptions on this plan (prod query 2026-06-09),
0 telemetry hits in 30d, ticket BILL-411 closed 2021.

Exercise 4 — Delete the boat anchor¶

Anti-pattern: Boat Anchor · Language: Go · Difficulty: ★★ medium

This package shipped a three-format exporter "so we're ready for anything." Two years on, only one format is used. Decide what is ballast and remove it — but do not amputate a real seam.

// exporter.go
type ReportExporter interface {
    ExportCSV(r Report) ([]byte, error)
    ExportXLSX(r Report) ([]byte, error)
    ExportXML(r Report) ([]byte, error)
}

type fileExporter struct{}

func (fileExporter) ExportCSV(r Report) ([]byte, error)  { /* 30 lines, used by /download */ }
func (fileExporter) ExportXLSX(r Report) ([]byte, error) { /* 80 lines, no caller */ }
func (fileExporter) ExportXML(r Report) ([]byte, error)  { /* 60 lines, no caller */ }

A grep shows: ExportCSV has one caller (an HTTP handler); ExportXLSX and ExportXML have zero callers and no tests.

Acceptance criteria - Unused, unrequested code is removed. - The remaining interface (if any) is justified — either it earns its keep today or it is collapsed. - You can articulate when you would have kept the interface.

Hint: an interface with one implementation and one method that one caller uses is not a seam — it is indirection. But check first whether the caller needs an interface for testing.

// exporter.go — only the format anyone actually uses survives.
func ExportCSV(r Report) ([]byte, error) {
    // ... the same 30 lines ...
}

type Exporter interface{ ExportCSV(Report) ([]byte, error) }
// handler holds an Exporter; tests pass a stub that returns an error.

Exercise 5 — Split the God Object¶

Anti-pattern: God Object · Language: Python · Difficulty: ★★ medium

OrderManager validates, prices, persists, and emails. Split it by responsibility.

class OrderManager:
    def __init__(self, db, smtp_host):
        self.db = db
        self.smtp_host = smtp_host
        self.tax_rate = 0.08

    def place_order(self, cart, customer):
        # validation
        if not cart.items:
            raise ValueError("empty cart")
        if customer.email is None:
            raise ValueError("no email")
        # pricing
        subtotal = sum(i.price * i.qty for i in cart.items)
        total = subtotal * (1 + self.tax_rate)
        # persistence
        order_id = self.db.insert("orders", {"customer": customer.id, "total": total})
        # notification
        body = f"Thanks {customer.name}, your order #{order_id} is ${total:.2f}"
        self._send_email(customer.email, "Order confirmed", body)
        return order_id

    def _send_email(self, to, subject, body):
        # ... opens an SMTP connection to self.smtp_host and sends ...
        ...

Acceptance criteria - Validation, pricing, persistence, and notification each live in their own focused unit. - OrderManager (renamed if appropriate) becomes a thin coordinator. - Each extracted unit is independently testable — no SMTP needed to test pricing.

Hint: every "# comment" in the original marks a responsibility boundary. Extract each, then have the coordinator call them in sequence.

class OrderValidator:
    def validate(self, cart, customer):
        if not cart.items:
            raise ValueError("empty cart")
        if customer.email is None:
            raise ValueError("no email")


class Pricer:
    def __init__(self, tax_rate=0.08):
        self.tax_rate = tax_rate

    def total(self, cart) -> float:
        subtotal = sum(i.price * i.qty for i in cart.items)
        return subtotal * (1 + self.tax_rate)


class OrderRepository:
    def __init__(self, db):
        self.db = db

    def save(self, customer_id, total) -> int:
        return self.db.insert("orders", {"customer": customer_id, "total": total})


class EmailSender:
    def __init__(self, smtp_host):
        self.smtp_host = smtp_host

    def send(self, to, subject, body):
        ...  # SMTP details isolated here


class CheckoutService:
    """Thin coordinator: it orchestrates, it does not implement."""
    def __init__(self, validator, pricer, repo, mailer):
        self._validator = validator
        self._pricer = pricer
        self._repo = repo
        self._mailer = mailer

    def place_order(self, cart, customer) -> int:
        self._validator.validate(cart, customer)
        total = self._pricer.total(cart)
        order_id = self._repo.save(customer.id, total)
        body = f"Thanks {customer.name}, your order #{order_id} is ${total:.2f}"
        self._mailer.send(customer.email, "Order confirmed", body)
        return order_id

Exercise 6 — Replace nesting with a handler map¶

Anti-pattern: Arrow + value dispatch · Language: Python · Difficulty: ★★ medium

This dispatcher grows an elif for every new command, and each branch is itself nested. Guard clauses alone will not fix it — the nesting is dispatch.

def handle(command: str, payload: dict) -> dict:
    if command == "create":
        if "name" in payload:
            return {"id": db_create(payload["name"])}
        else:
            return {"error": "name required"}
    elif command == "delete":
        if "id" in payload:
            db_delete(payload["id"])
            return {"ok": True}
        else:
            return {"error": "id required"}
    elif command == "rename":
        if "id" in payload and "name" in payload:
            db_rename(payload["id"], payload["name"])
            return {"ok": True}
        else:
            return {"error": "id and name required"}
    else:
        return {"error": f"unknown command {command}"}

Acceptance criteria - Adding a new command does not require editing a long if/elif chain. - Each command's logic lives in its own small function. - Unknown commands are handled in one place.

Hint: map the command string to a handler function. Keep per-handler validation inside the handler (guard clauses), and the dispatch in a single table lookup.

def _require(payload, *keys):
    missing = [k for k in keys if k not in payload]
    if missing:
        raise ValueError(f"{', '.join(missing)} required")


def _create(payload):
    _require(payload, "name")
    return {"id": db_create(payload["name"])}


def _delete(payload):
    _require(payload, "id")
    db_delete(payload["id"])
    return {"ok": True}


def _rename(payload):
    _require(payload, "id", "name")
    db_rename(payload["id"], payload["name"])
    return {"ok": True}


HANDLERS = {"create": _create, "delete": _delete, "rename": _rename}


def handle(command: str, payload: dict) -> dict:
    handler = HANDLERS.get(command)
    if handler is None:
        return {"error": f"unknown command {command}"}
    try:
        return handler(payload)
    except ValueError as e:
        return {"error": str(e)}

Exercise 7 — Untangle the shared-state pipeline¶

Anti-pattern: Spaghetti (shared mutable state, order-dependent calls) · Language: Python · Difficulty: ★★ medium

These functions communicate through a module-level dict and only work if called in a secret order. Convert them into an explicit pipeline.

_state = {}

def load(path):
    _state["raw"] = open(path).read()
    _state["loaded"] = True

def clean():
    # crashes if load() didn't run
    _state["rows"] = [line.strip() for line in _state["raw"].splitlines() if line.strip()]

def parse():
    if not _state.get("loaded"):
        raise RuntimeError("call load() first")   # the secret contract, made loud
    _state["records"] = [r.split(",") for r in _state["rows"]]

def summary():
    return {"count": len(_state["records"]), "first": _state["records"][0]}

# usage — order matters and is invisible:
# load("data.csv"); clean(); parse(); print(summary())

Acceptance criteria - No module-level mutable state. - Each step takes its input as an argument and returns its output. - It is impossible to call a step "out of order" because the data simply would not exist.

Hint: the call order is the data flow. Encode it by passing each step's return value into the next.

def load(path: str) -> str:
    with open(path) as f:
        return f.read()

def clean(raw: str) -> list[str]:
    return [line.strip() for line in raw.splitlines() if line.strip()]

def parse(rows: list[str]) -> list[list[str]]:
    return [r.split(",") for r in rows]

def summarize(records: list[list[str]]) -> dict:
    return {"count": len(records), "first": records[0]}

def run(path: str) -> dict:
    return summarize(parse(clean(load(path))))

Exercise 8 — Replace type-switch with polymorphism¶

Anti-pattern: Arrow + type dispatch · Language: Java · Difficulty: ★★ medium

A nested instanceof ladder computes area. Every new shape adds another branch in every such method. Push the behavior into the types.

double area(Shape s) {
    if (s instanceof Circle) {
        Circle c = (Circle) s;
        if (c.radius() > 0) {
            return Math.PI * c.radius() * c.radius();
        } else {
            return 0;
        }
    } else if (s instanceof Rectangle) {
        Rectangle r = (Rectangle) s;
        if (r.width() > 0 && r.height() > 0) {
            return r.width() * r.height();
        } else {
            return 0;
        }
    }
    throw new IllegalArgumentException("unknown shape");
}

Acceptance criteria - Adding a new shape does not require editing area. - Each shape computes its own area. - The caller is reduced to a single call.

Hint: this is the textbook Replace Conditional with Polymorphism. Give Shape an area() method and let each subtype implement it.

sealed interface Shape permits Circle, Rectangle {
    double area();
}

record Circle(double radius) implements Shape {
    public double area() {
        return radius > 0 ? Math.PI * radius * radius : 0;
    }
}

record Rectangle(double width, double height) implements Shape {
    public double area() {
        return (width > 0 && height > 0) ? width * height : 0;
    }
}

// caller:
double a = shape.area();

Exercise 9 — Inject the collaborators¶

Anti-pattern: God Object (hidden dependencies) · Language: Go · Difficulty: ★★★ hard

This UserService constructs its own dependencies inside its methods, so it cannot be tested without a real database, a real clock, and a real mailer. Refactor it for dependency injection and split a responsibility that does not belong.

type UserService struct{}

func (UserService) Register(email, password string) error {
    // builds its own DB connection — untestable
    db, err := sql.Open("postgres", os.Getenv("DATABASE_URL"))
    if err != nil {
        return err
    }
    defer db.Close()

    hash := sha256.Sum256([]byte(password)) // also: weak hashing, but out of scope here
    _, err = db.Exec("INSERT INTO users(email, hash, created) VALUES($1,$2,$3)",
        email, hex.EncodeToString(hash[:]), time.Now()) // time.Now() is non-deterministic in tests
    if err != nil {
        return err
    }

    // and it sends a welcome email inline
    smtp.SendMail(os.Getenv("SMTP_ADDR"), nil, "noreply@app.com",
        []string{email}, []byte("Welcome!"))
    return nil
}

Acceptance criteria - Dependencies (store, clock, mailer) are injected via the constructor, not built inside the method. - time.Now() is replaced by an injected clock so the method is deterministic under test. - Email sending is a separate collaborator, not inline SMTP. - The method can be unit-tested with fakes and no network or database.

Hint: define small interfaces for each external concern. UserService should hold them, not create them.

type UserStore interface {
    Insert(email, hash string, created time.Time) error
}

type Mailer interface {
    SendWelcome(email string) error
}

type Clock interface {
    Now() time.Time
}

type UserService struct {
    store  UserStore
    mailer Mailer
    clock  Clock
}

func NewUserService(s UserStore, m Mailer, c Clock) *UserService {
    return &UserService{store: s, mailer: m, clock: c}
}

func (u *UserService) Register(email, password string) error {
    hash := sha256.Sum256([]byte(password))
    if err := u.store.Insert(email, hex.EncodeToString(hash[:]), u.clock.Now()); err != nil {
        return fmt.Errorf("register %s: %w", email, err)
    }
    if err := u.mailer.SendWelcome(email); err != nil {
        return fmt.Errorf("welcome mail %s: %w", email, err)
    }
    return nil
}

type fakeStore struct{ inserted bool; created time.Time }
func (f *fakeStore) Insert(_, _ string, t time.Time) error { f.inserted = true; f.created = t; return nil }

type fakeMailer struct{ sent string }
func (f *fakeMailer) SendWelcome(e string) error { f.sent = e; return nil }

type fixedClock struct{ t time.Time }
func (c fixedClock) Now() time.Time { return c.t }

func TestRegister(t *testing.T) {
    store, mailer := &fakeStore{}, &fakeMailer{}
    svc := NewUserService(store, mailer, fixedClock{t: time.Unix(0, 0)})

    if err := svc.Register("a@b.com", "pw"); err != nil {
        t.Fatal(err)
    }
    if !store.inserted || mailer.sent != "a@b.com" || !store.created.Equal(time.Unix(0, 0)) {
        t.Fatalf("unexpected state: %+v %+v", store, mailer)
    }
}

Exercise 10 — Tame the order-dependent state machine¶

Anti-pattern: Spaghetti (implicit state + order dependence) · Language: Python · Difficulty: ★★★ hard

A checkout flow is driven by scattered boolean flags. Any method can be called at any time, and illegal transitions corrupt state silently. Replace the flags with an explicit state machine that rejects illegal transitions.

class Checkout:
    def __init__(self):
        self.cart_open = True
        self.paid = False
        self.shipped = False

    def pay(self):
        # nothing stops you paying twice, or paying after shipping
        self.paid = True
        self.cart_open = False

    def ship(self):
        # you can ship without paying — silent bug
        self.shipped = True

    def add_item(self, item):
        # you can add items after payment — silent bug
        if self.cart_open:
            ...

Acceptance criteria - Replace the three booleans with a single explicit state. - Each operation is legal only from the correct state(s); an illegal call raises a clear error. - The legal transitions are visible in one place.

Hint: model states as an enum (OPEN → PAID → SHIPPED) and define a single transition table. Each method asserts its allowed source state before transitioning.

from enum import Enum, auto

class State(Enum):
    OPEN = auto()
    PAID = auto()
    SHIPPED = auto()

# The legal transitions, in one readable place.
_TRANSITIONS = {
    State.OPEN:    {State.PAID},
    State.PAID:    {State.SHIPPED},
    State.SHIPPED: set(),
}

class Checkout:
    def __init__(self):
        self.state = State.OPEN
        self.items = []

    def _transition(self, target: State):
        if target not in _TRANSITIONS[self.state]:
            raise ValueError(f"cannot go {self.state.name} -> {target.name}")
        self.state = target

    def add_item(self, item):
        if self.state is not State.OPEN:
            raise ValueError(f"cannot add items in state {self.state.name}")
        self.items.append(item)

    def pay(self):
        if not self.items:
            raise ValueError("cannot pay for an empty cart")
        self._transition(State.PAID)

    def ship(self):
        self._transition(State.SHIPPED)   # only reachable from PAID

Exercise 11 — Lava Flow archaeology — evidence then delete¶

Anti-pattern: Lava Flow + Boat Anchor · Difficulty: ★★★ hard · (process exercise — no single code answer)

You inherit a 4,000-line service. Three suspicious things catch your eye:

1. A legacy/ package, imported by nothing you can find, with a comment: // keep — billing integration, do not touch.
2. A method recomputeShadowTotals() called from one place, inside if featureFlags.shadowMode { ... } — and shadowMode is hard-coded false.
3. A public function ExportToMainframe() with no callers in this repo, but it is exported, so another repo might use it.

Task. For each item, write the concrete evidence-gathering plan, decide delete-vs-keep, and describe how you would stage the change safely. Be specific about tools and commands.

Acceptance criteria - A distinct investigation per item (they need different evidence). - An explicit decision with the evidence that justifies it. - A safe rollout: separate commits, reversibility, what you would watch after merging.

Exercise 12 — Mini-project: refactor the ReportTool¶

Anti-pattern: all five, in one small realistic module · Language: Python · Difficulty: ★★★★ project

Below is a compact module that manages to combine every bad-structure anti-pattern: a God Object that does everything, spaghetti via a flag and shared state, arrow nesting, a dead branch (Lava Flow), and an unused method (Boat Anchor). Refactor it into clean, testable units. Work in steps; do not try to fix it all in one edit.

class ReportTool:
    def __init__(self, db):
        self.db = db
        self.cache = {}            # shared mutable state used across methods
        self.last_format = None    # set by one method, read by another (spaghetti)

    def generate(self, kind, raw_format, send_email, debug):
        # arrow + flags + dead branch all at once
        if kind is not None:
            if kind in ("sales", "inventory"):
                rows = self.db.query(kind)
                if rows:
                    if raw_format == "csv":
                        out = "\n".join(",".join(map(str, r)) for r in rows)
                        self.last_format = "csv"
                    elif raw_format == "json":
                        import json
                        out = json.dumps(rows)
                        self.last_format = "json"
                    elif raw_format == "xml_v0":
                        # legacy. broke once in 2020. nobody calls with xml_v0 anymore (right?)
                        out = "<xml>%s</xml>" % rows
                    else:
                        return None
                    self.cache[kind] = out
                    if send_email and not debug:
                        self._email("ops@co.com", out)
                    return out
                else:
                    return None
            else:
                return None
        return None

    def _email(self, to, body):
        # inline SMTP, untestable
        ...

    def export_to_ftp(self, out):
        # never called anywhere in the codebase (boat anchor)
        ...

Acceptance criteria - God Object: querying, formatting, caching, and emailing are separated. - Spaghetti: the send_email/debug flags and the last_format/cache shared state are removed or made explicit. - Arrow: the body uses guard clauses; format selection is a dispatch, not an elif ladder. - Lava Flow: the xml_v0 branch is removed (assume you gathered evidence as in Exercise 3 and 11). - Boat Anchor: export_to_ftp is deleted (assume confirmed zero callers). - Each remaining unit is testable in isolation.

Hint: start by listing the responsibilities, then extract them one at a time — query, format (via a formatter map), email — keeping behavior green after each step. Decide what cache and last_format were actually for (probably nothing essential).

import json

# --- Formatting: a dispatch map replaces the elif ladder; xml_v0 (dead) is gone. ---
def _to_csv(rows) -> str:
    return "\n".join(",".join(map(str, r)) for r in rows)

def _to_json(rows) -> str:
    return json.dumps(rows)

FORMATTERS = {"csv": _to_csv, "json": _to_json}

def format_rows(rows, fmt: str) -> str:
    formatter = FORMATTERS.get(fmt)
    if formatter is None:
        raise ValueError(f"unsupported format {fmt!r}")
    return formatter(rows)


# --- Data access: one job. ---
class ReportRepository:
    VALID_KINDS = {"sales", "inventory"}

    def __init__(self, db):
        self.db = db

    def rows_for(self, kind: str):
        if kind not in self.VALID_KINDS:
            raise ValueError(f"unknown report kind {kind!r}")
        return self.db.query(kind)


# --- Notification: a separate, injectable collaborator. ---
class EmailSender:
    def send(self, to: str, body: str) -> None:
        ...  # SMTP isolated here, mockable in tests


# --- Coordinator: guard clauses, no flags, no shared state. ---
class ReportService:
    def __init__(self, repo: ReportRepository, mailer: EmailSender):
        self._repo = repo
        self._mailer = mailer

    def generate(self, kind: str, fmt: str) -> str | None:
        rows = self._repo.rows_for(kind)      # raises on bad kind (guard at the source)
        if not rows:
            return None
        return format_rows(rows, fmt)

    def generate_and_email(self, kind: str, fmt: str, to: str) -> str | None:
        report = self.generate(kind, fmt)
        if report is None:
            return None
        self._mailer.send(to, report)
        return report

Exercise 13 — Write a structure review checklist¶

Anti-pattern: meta (prevention) · Difficulty: ★★ medium

Bad structure is cheapest to stop in code review, while the change is still small. Write a concise, actionable reviewer checklist — phrased as questions a reviewer asks of a diff — that would catch all five anti-patterns before they merge. Aim for questions with a clear "if yes, push back" trigger, not vague advice like "is this clean?"

Acceptance criteria - One or more concrete, answerable questions per anti-pattern. - Each question has a clear failure trigger and a suggested action. - Short enough that a reviewer would actually run it on every PR.

Exercise 14 — Write a fitness test that fails the build¶

Anti-pattern: meta (automated enforcement) · Language: Go · Difficulty: ★★★ hard

A checklist relies on humans remembering. A fitness test (a.k.a. architecture test) encodes a structural rule as an automated test that fails CI when violated. Write a Go test that enforces a simple anti-God-Object rule: no .go file in the service package may exceed 400 lines. (Line count is a crude proxy, but it is an objective tripwire that flags files for human review.)

Acceptance criteria - The test walks the package's source files and counts lines. - It fails with a clear message naming the offending file(s) and their size. - It is fast and has no external dependencies (pure standard library). - Note in a comment why line-count is a smoke detector, not a verdict.

Hint: use os.ReadDir + bufio.Scanner (or bytes.Count). Skip _test.go files. Collect all violations before failing so one run reports every offender.

package service

import (
    "bytes"
    "os"
    "path/filepath"
    "strings"
    "testing"
)

// maxLines is a smoke detector for God Objects, not a verdict: a cohesive
// 450-line file can be fine, but crossing the line forces a human to look.
const maxLines = 400

func TestNoOversizedFiles(t *testing.T) {
    entries, err := os.ReadDir(".")
    if err != nil {
        t.Fatalf("read package dir: %v", err)
    }

    var violations []string
    for _, e := range entries {
        name := e.Name()
        if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
            continue
        }
        data, err := os.ReadFile(filepath.Join(".", name))
        if err != nil {
            t.Fatalf("read %s: %v", name, err)
        }
        lines := bytes.Count(data, []byte{'\n'}) + 1
        if lines > maxLines {
            violations = append(violations, name+" has "+itoa(lines)+" lines")
        }
    }

    if len(violations) > 0 {
        t.Fatalf("files exceed %d-line limit (likely God Objects — split by responsibility):\n  %s",
            maxLines, strings.Join(violations, "\n  "))
    }
}

func itoa(n int) string { return strconv.Itoa(n) } // import "strconv"

--- FAIL: TestNoOversizedFiles
    fitness_test.go:34: files exceed 400-line limit (likely God Objects — split by responsibility):
      order_manager.go has 612 lines

Summary¶

• These exercises move you from recognizing bad structure to removing it: flatten arrows into guard clauses, turn dispatch nesting into handler maps and polymorphism, kill flag-arguments and shared state with explicit data flow, split God Objects by responsibility and inject collaborators, and prove dead code dead before deleting it.
• Refactor in small green steps. Every multi-part exercise (5, 9, 10, 12) is done as a sequence — extract one thing, keep it working, repeat — never as a big-bang rewrite. Pin behavior with a test first; keep structural and behavioral changes in separate commits.
• Deletion is a refactoring tool, and git is the safety net. Lava Flow and Boat Anchor are cured by evidence then delete, not by tidying.
• Prevention scales better than cure. The meta-exercises (13, 14) move the fight upstream — a review checklist and an automated fitness test catch bad structure while the diff is still small and cheap to fix.
• The five anti-patterns travel together, so a real module (Exercise 12) usually shows several at once. Fix the one in the file you are editing today, and the gravity that pulls in the others weakens.

Related Topics¶

• junior.md — the five shapes and how to recognize them on sight.
• middle.md — the forces that create these shapes and the countermoves used here.
• find-bug.md — spot-the-anti-pattern snippets (critical reading practice).
• optimize.md — more flawed implementations to clean up.
• interview.md — Q&A across all levels for job prep.
• Refactoring → Refactoring Techniques — Extract Class, Replace Conditional with Polymorphism, Replace Nested Conditional with Guard Clauses.
• Clean Code → Classes — the SRP cure for God Objects.
• Clean Code → Functions — small functions, flag arguments, guard clauses.
• Over-Engineering — the sibling category; Boat Anchor and Lasagna live next door.