Capstone II: the incremental refactor

Net first: characterization tests pin what the module does today, before you understand why. You do not refactor untested legacy code by reading it harder. You pin it. A characterization test asserts the module’s current observed behavior — not the behavior you wish it had — so that any later structural move that changes output turns a test red instantly.

// characterize.test.ts — written BEFORE any structural change.
// We don't yet trust the rounding or the fee math; we record what it DOES.
test("card order total with tax and fee", () => {
  const out = processOrder({ items: [{ price: 1000, qty: 2 }], paymentType: "card" });
  expect(out.total).toBe(2070);   // 2000 + 5% tax + 2% card fee, as-is
  expect(out.currency).toBe("USD");
});

test("invoice order skips the card fee", () => {
  const out = processOrder({ items: [{ price: 999, qty: 3 }], paymentType: "invoice" });
  expect(out.total).toBe(3147);   // whatever it currently computes — pin it
});

The number 2070 might encode a bug. That is fine — you are not fixing it now, you are recording it. A characterization test that locks in a wrong value is still doing its job: it guarantees your refactor preserves behavior. Fix the bug later, as its own commit, after the structure is clean enough to fix it safely. No structural edit happens until the net is green.

Extract under green, one move per commit — and commit the message that says “refactor”. With the net in place, the first moves are the cheap, mechanical ones from the catalog. Pull the tangled blocks of processOrder into named methods, then lift the cohesive cluster into a class. Each is its own commit; the suite is green before and after every one.

// commit "refactor: extract calcTax / calcFee"  — behavior identical
function calcTax(subtotal: number): number { return Math.round(subtotal * 0.05); }
function calcFee(subtotal: number, paymentType: string): number {
  return paymentType === "card" ? Math.round(subtotal * 0.02) : 0;
}

// commit "refactor: extract OrderProcessor class"  — same body, new home
class OrderProcessor {
  process(order: OrderInput): OrderResult {
    const subtotal = order.items.reduce((s, i) => s + i.price * i.qty, 0);
    const tax = calcTax(subtotal);
    const fee = calcFee(subtotal, order.paymentType);
    return { total: subtotal + tax + fee, currency: "USD" };
  }
}

Nothing here is clever, and that is the point. These are behavior-preserving moves; the green net proves it after each. If a test goes red, you reverted one small commit, not a week of work — the blast radius of any mistake is exactly one move.

Introduce value objects to retire primitive obsession — still no behavior change. The module passes raw numbers for money and raw strings for ids; that is primitive obsession, and it is why rounding and currency logic leaked everywhere. Wrap them. The move is still a refactor: Money must compute the same numbers the net already pinned.

// commit "refactor: introduce Money value object"
class Money {
  private constructor(readonly cents: number, readonly currency: "USD") {}
  static usd(cents: number): Money { return new Money(Math.round(cents), "USD"); }
  add(o: Money): Money { return Money.usd(this.cents + o.cents); }
  percent(p: number): Money { return Money.usd(Math.round(this.cents * p)); }
}

// commit "refactor: introduce OrderId" — a typed id, no more bare string
class OrderId { private constructor(readonly value: string) {}
  static of(v: string): OrderId { return new OrderId(v); } }

Because Money.usd rounds exactly where the old code did, the pinned 2070 still holds — the test stays green. The win is that rounding now lives in one place instead of being re-derived at every call site, which is what makes the next change cheap. Crucially, you did not “fix” the rounding while you were in there; if the rounding is wrong, that is a separate, later, behavior-changing commit.

Replace the payment-type switch with polymorphism, then invert the persistence dependency behind a port. Two structural moves remain, and they are the high-value ones. First, the paymentType switch that fans out across the module becomes a small strategy hierarchy — Open/Closed: a new payment type is a new class, not an edit to a switch in five files.

// commit "refactor: replace paymentType switch with PaymentMethod polymorphism"
interface PaymentMethod { fee(subtotal: Money): Money; }
class Card    implements PaymentMethod { fee(s: Money) { return s.percent(0.02); } }
class Invoice implements PaymentMethod { fee(s: Money) { return Money.usd(0); } }
const methods: Record<string, PaymentMethod> = { card: new Card(), invoice: new Invoice() };

Then invert the persistence dependency. OrderProcessor currently imports a concrete db; that is the wrong-way arrow. Define a OrderRepository port the processor owns, and make the database an adapter that implements it — the high-level policy stops depending on the low-level detail.

// commit "refactor: introduce OrderRepository port"
interface OrderRepository { save(id: OrderId, result: OrderResult): Promise<void>; }
// adapter lives at the edge; processor depends on the interface, not on `db`
class PostgresOrderRepository implements OrderRepository { /* the old SQL, moved */ }

Both moves are behavior-preserving: same fees, same persisted rows, same green net. They only change who depends on whom.