Legacy code & the boy-scout rule

Legacy code is simply code without tests — and that definition tells you what to do. Michael Feathers’ working definition strips the word of its emotion: legacy isn’t “old” or “someone else’s” or “in a language I dislike.” It’s code you cannot change with confidence because nothing tells you when you’ve broken it. Brand-new code you wrote this morning with no tests is already legacy by this measure.

The point of the definition is operational. If the problem is “no safety net,” the solution is not “be careful” or “read harder” — it’s get a net under it first. Everything else in this lesson follows from that. You don’t refactor legacy code and then test it; you test it and then you can refactor it.

Before you can test it, you need a seam — a place to alter behaviour without editing the code in that spot. The reason legacy code is hard to test is usually that it wires its own collaborators: it news a database client, calls a static singleton, reads Date.now() directly. A seam is the point where you can substitute a fake. Often the cheapest seam is dependency injection at the boundary you already need to touch.

// No seam: the dependency is welded in, so the branch can't be tested in isolation.
class OrderService {
  price(order: Order): number {
    const now = new Date();                 // welded to wall-clock time
    const fx = new FxClient().rateFor(order.currency); // welded to network
    return applyRules(order, now, fx);
  }
}

// A seam introduced by parameterizing the boundary — behaviour identical today.
class OrderService {
  constructor(private clock: () => Date, private fx: FxRates) {}
  price(order: Order): number {
    return applyRules(order, this.clock(), this.fx.rateFor(order.currency));
  }
}

You haven’t changed any rules. You’ve made the code substitutable, which is the precondition for pinning its behaviour.

Pin the current behaviour with characterization tests — not “correct” behaviour, current behaviour. A characterization test documents what the code actually does today, bugs included. You don’t assert what the price should be; you run the code, observe what it returns, and freeze that as the expected value. The test’s job is not correctness — it’s a tripwire that fires the moment your edit changes any observable output.

// You didn't decide 47.5 is "right" — the code produced it. You pinned it.
test("characterize: GBP order at 2021 rates", () => {
  const svc = new OrderService(() => new Date("2021-06-01"), fakeRates({ GBP: 1.18 }));
  expect(svc.price(gbpOrder)).toBe(47.5); // whatever it actually returns, locked in
});

If a characterization test surfaces what looks like a bug, you do not fix it now. You pin the buggy output, ship your real change, and fix the bug as a separate, visible change with its own test. Conflating “make my change” with “fix the bug I found” is how a one-line change becomes an unreviewable diff.

Now make the change, then boy-scout only inside the blast radius you’ve already touched. With the net in place, your real edit is safe: if you break pinned behaviour, a test goes red. The boy-scout rule — always leave the code a little cleaner than you found it — is the incremental strategy that, repeated across hundreds of commits, drags a codebase off the high-cost end of the change curve without anyone ever scheduling a “cleanup sprint.”

The senior qualifier is the word opportunistic. You improve the code you’re already reading and already covering with tests — rename the misleading variable in the function you’re editing, extract the duplicated branch you just had to understand. You do not open neighbouring files to “tidy them while you’re here.” The cleanup must ride inside the change’s existing blast radius. The moment it leaves that radius, it’s a new piece of work that needs its own justification, its own tests, and its own review — and folding it into this diff just makes both changes harder to review and riskier to ship.