Working with legacy code

Most professional programming is not writing new code on a blank page. It is changing code that already exists, was written by someone else, possibly years ago, often under deadline, and usually without the documentation or tests you would like. Michael Feathers, in his book Working Effectively with Legacy Code, gave a blunt definition: legacy code is simply code without tests. By that definition, Ridgeline's process_order, written in a hurry two years ago and never tested, is legacy code, even though Jordan wrote it.

The earlier lessons showed what good design looks like. This lesson is about getting there from where you actually are, safely, without a risky rewrite and without stopping the business. Ridgeline cannot close the online shop for a month while Jordan redesigns it. The techniques here, characterization tests, seams, sprouting and wrapping new code, small refactorings and the strangler fig, let a team improve a design one safe step at a time while continuing to ship features.

The temptation to rewrite

Faced with process_order, Sam's first suggestion was to throw it away and write a clean version from scratch. Lena told him why that rarely works. The old code, however ugly, contains years of small decisions and fixes: the rounding rule the accountant insisted on, the special case for the Alder Hotel's net-60 terms, the workaround for the payment provider's habit of timing out on Mondays. Many of those decisions are written down nowhere else. A rewrite tends to lose them, takes longer than planned, and must chase a moving target as the old system keeps changing.

Joel Spolsky called the full rewrite the single worst strategic mistake a software company can make. That is too strong as a rule, since sometimes replacement really is right, but the usual better path is incremental: put tests around the existing behavior, improve the structure in small steps, and replace parts gradually while the system keeps running.

The legacy change algorithm

Feathers describes a routine for changing legacy code safely. First, identify the change points: the places where the change must be made. Second, find test points: places where you can observe the behavior of the code around the change. Third, break dependencies that prevent you from getting that code into a test, such as a hard-coded database connection or a call to the real clock. Fourth, write tests that pin down the current behavior. Fifth, make the change and refactor, with the tests telling you whether anything else changed.

The third step is where most of the difficulty lies, and the fourth is where most people skip ahead. Without tests, every refactoring is a gamble; with them, the structure can change boldly because mistakes show up in seconds.

Characterization tests

When you do not know exactly what code is supposed to do, you can still record what it does. A characterization test, sometimes called a golden master or snapshot test, runs the existing code on a range of inputs and records the outputs, then asserts that the code keeps producing them. It documents actual behavior, not intended behavior, including any bugs, because the first goal is to change the structure without changing the behavior. Bugs found along the way are noted and fixed afterwards, as separate, deliberate changes.

def legacy_total(kind, qty, price, code=""):
    t = qty * price
    if kind == "W":
        t = t - t * 15 / 100
    if kind == "O" and code == "WELCOME10":
        t = t * 0.9
    if kind == "O" and t < 50:
        t = t + 8.95
    return round(t, 2)

cases = [("C", 2, 18.5), ("O", 1, 18.5), ("O", 3, 18.5), ("W", 10, 18.5),
         ("O", 1, 18.5, "WELCOME10"), ("O", 3, 18.5, "WELCOME10"), ("X", 1, 18.5)]
golden = {case: legacy_total(*case) for case in cases}
for case, result in golden.items():
    print(case, "->", result)

('C', 2, 18.5) -> 37.0
('O', 1, 18.5) -> 27.45
('O', 3, 18.5) -> 55.5
('W', 10, 18.5) -> 157.25
('O', 1, 18.5, 'WELCOME10') -> 25.6
('O', 3, 18.5, 'WELCOME10') -> 58.9
('X', 1, 18.5) -> 18.5

Look closely at the output and you will find surprises worth recording: an unknown customer type X is silently treated like a cafe customer, and the promotion code is applied before the shipping threshold, so a discount can push an order below 50 and add a shipping fee. Rosa may decide those are bugs, but that is a separate decision. The golden master makes sure the refactoring does not change them by accident.

def new_total(kind, qty, price, code=""):
    t = qty * price
    if kind == "W":
        t *= 0.85
    if kind == "O":
        if t < 50:
            t += 8.95
        if code == "WELCOME10":
            t *= 0.9
    return round(t, 2)

Seams: places to change behavior without editing

Feathers defines a seam as a place where you can alter behavior without editing the code in that place. Seams are how you get legacy code into a test: you replace a dependency that would send real emails, charge real cards or read the real clock with something the test controls. Python offers several kinds. A parameter seam adds a parameter with a default, as in lesson 2's clock injection. An object seam passes in an object whose methods can be replaced. A module seam replaces a module-level name during a test, which is what unittest.mock.patch and pytest's monkeypatch do.

Module seams are the quickest way to start testing untouchable code, but they test the code as it is, tangled dependencies and all. Parameter and object seams improve the design as they go, because they make dependencies explicit. A good habit is to use patching to get the first characterization tests in place, then introduce proper parameters and remove the patches as the code improves.

import datetime

SENT = []

def send_email(to, text):          # imagine this talks to a real mail server
    SENT.append((to, text))

def remind_stale_bags(bags):
    """Legacy: reads the real clock and sends real email."""
    for bag in bags:
        if (datetime.date.today() - bag["roasted"]).days > 30:
            send_email("kai@ridgeline.example", f"{bag['sku']} is past its sell-by window")

# A module seam for a first test: replace send_email with a fake while the test runs.
from unittest import mock
captured = []
with mock.patch(f"{__name__}.send_email", lambda to, text: captured.append(text)):
    remind_stale_bags([{"sku": "RC-104", "roasted": datetime.date(2000, 1, 1)}])
print(captured)
print(SENT, "(the real send_email was never called)")

['RC-104 is past its sell-by window']
[] (the real send_email was never called)

from datetime import date

OUTBOX = []

def send_email(to, text):
    OUTBOX.append((to, text))

def remind_stale(bags):
    count = 0
    for bag in bags:
        if (date.today() - bag["roasted"]).days > 30:
            send_email("kai@ridgeline.example", f"{bag['sku']} is past its sell-by window")
            count += 1
    return count

Sprout and wrap: adding features without making it worse

Sometimes you must add a feature to legacy code before you can test or refactor it properly. Two techniques keep the new code clean even when the surrounding code is not. Sprout method: write the new behavior as a new, fully tested function, and add one line in the legacy code to call it. Wrap method: rename the old function, and create a new function with the old name that calls it and adds the new behavior before or after. Both keep the new logic out of the tangle, so the tangle does not grow and the new code starts life with tests.

When Rosa asked for gift wrapping at 4.50 per wrapped item, Jordan did not add another branch inside process_order. He wrote gift_wrap_fee(items) as a pure function with its own tests, then added one line to process_order that adds its result to the total. When process_order is finally refactored, the fee function will move into the new design unchanged.

PRICES = {"RC-101": 18.50, "RC-102": 22.00, "RC-104": 20.25}

def checkout_total(items):
    subtotal = 0
    for sku, qty, wrapped in items:
        subtotal = subtotal + PRICES[sku] * qty
    return round(subtotal, 2)

Small, safe refactorings

With tests in place, structure improves through a sequence of small refactorings, each of which keeps the code working. Martin Fowler's book Refactoring catalogs dozens; a handful do most of the work. Extract function pulls a coherent block into a named function. Rename gives a variable or function a name that says what it means. Introduce parameter object replaces a long list of parameters that travel together with a dataclass. Replace conditional with polymorphism or a dictionary turns a repeated if-chain on a type code into strategies. Move function puts code next to the data it uses.

Each step should be small enough that, if the tests fail, the cause is obvious, and each should be a separate commit in version control with a clear message, so any step can be reviewed or undone on its own. Editors and tools such as PyCharm's refactoring menu or the rope library can perform renames and extractions automatically and safely across a whole project.

The strangler fig

For replacing large parts of a system, Martin Fowler borrowed an image from rainforests: the strangler fig, a vine that grows around a host tree, gradually taking over until the old tree can be removed. Applied to software, you put a thin routing layer in front of the old code, build new functionality beside it, and send more and more requests to the new code until nothing reaches the old code and it can be deleted. At every point the system works, and the riskiest moment of a big-bang switch-over never happens.

Ridgeline's checkout uses this approach. A small function, checkout(order), first sent only wholesale orders to the new OrderService and everything else to process_order. Once wholesale had run without problems for two weeks, online orders followed, then cafe sales, and finally process_order was deleted in a commit that Jordan describes as the most satisfying of his career.

def process_order_legacy(order):
    return f"legacy handled {order['kind']} order {order['id']}"

def new_order_service(order):
    return f"new service handled {order['kind']} order {order['id']}"

MIGRATED = {"wholesale"}                 # grows as each kind is moved over

def checkout(order):
    """The strangler: one routing point in front of old and new code."""
    if order["kind"] in MIGRATED:
        return new_order_service(order)
    return process_order_legacy(order)

orders = [{"id": 2101, "kind": "wholesale"}, {"id": 2102, "kind": "online"}]
for o in orders:
    print(checkout(o))
MIGRATED.add("online")
print(checkout(orders[1]))

new service handled wholesale order 2101
legacy handled online order 2102
new service handled online order 2102