working-with-legacy-code
verifiedad703330-37a0-4fdb-a117-6e47392acd1f
Use when modifying code without tests — characterization tests pin current behavior as a golden baseline, find seams for safe injection, and keep edits minimal and localized.
Metadata
Skill file
# Working with Legacy Code
**Use when** modifying code that has no tests and no clear structure. The goal: make the change safely WITHOUT a full rewrite. First rule of legacy code: don't break what works.
## The Core Technique: Characterization Tests
A characterization test pins CURRENT behavior — even if it looks "wrong" — as a golden baseline. It doesn't assert correctness; it asserts "this is what the code does today."
```python
# Legacy function with NO tests and unclear behavior:
def calculate_fee(amount, customer_type):
if customer_type == "corporate":
return amount * 0.05
if customer_type == "vip":
return amount * 0.01
# No else — what happens for "regular"? Let's find out...
# Characterization test: PIN what it does TODAY
def test_calculate_fee_characterizes_regular():
# We don't KNOW what the "right" answer is, but this pins the current behavior
assert calculate_fee(100, "regular") is None # discovered: returns None
# This test now guards against accidental behavior change during refactoring.
# It's not "correct" — but it's the current reality, and that's what matters first.
```
### How to write characterization tests
```text
1. Call the legacy function with a variety of inputs
2. RECORD the outputs (even if surprising)
3. Assert those exact outputs
4. Now you have a safety net for refactoring
```
### Golden Master technique (for large outputs)
```python
# Pin a large/complex output as a snapshot
def test_report_golden_master():
report = generate_legacy_report(sample_data())
# Snapshot the whole thing; diff on any change
with open("tests/golden/report.json") as f:
expected = f.read()
assert json.dumps(report, indent=2) == expected
```
## Finding Seams
A seam is a place where you can inject behavior without restructuring the whole system. Find them before you edit.
### Common seams in legacy code
| Seam Type | How to use it |
|-----------|---------------|
| **Function parameter** | Add a default arg (`clock=None`, `http=None`) |
| **Module-level variable** | Replace a module global with an injectable one |
| **Class constructor** | Inject a dependency via `__init__` |
| **Config/settings object** | Route the dependency through config |
| **Inheritance** | Subclass and override the method you need to change |
```python
# Legacy code with a hard-coded dependency (no seam):
import requests
def get_user_status(user_id):
resp = requests.get(f"https://api.example.com/user/{user_id}")
return resp.json()["status"]
# CREATE A SEAM — add a default param WITHOUT changing behavior:
def get_user_status(user_id, http=None):
http = http or requests # default to real thing
resp = http.get(f"https://api.example.com/user/{user_id}")
return resp.json()["status"]
# Now you can test it by injecting a fake http client.
```
## Slicing a Change (Minimal & Localized)
When you must change legacy code, keep the edit surgical:
```text
1. Add a characterization test around the EXACT lines you'll touch
2. Make the smallest possible edit (a few lines, not a rewrite)
3. Re-run the characterization test + the full suite
4. Commit the change + the new test together
```
```python
# Example: you need to fix a bug in line 3, don't touch anything else
def process(data):
result = normalize(data) # line 1 — DON'T TOUCH
result = deduplicate(result) # line 2 — DON'T TOUCH
total = sum(result) + 1 # line 3 — THE BUG (off-by-one)
return total
# Characterization test FIRST (pins current buggy behavior):
def test_process_current_behavior():
assert process([1, 2, 3]) == 7 # 1+2+3+1 = 7 (the bug)
# Then fix ONLY line 3:
def process(data):
result = normalize(data)
result = deduplicate(result)
total = sum(result) # line 3 — FIXED (removed +1)
return total
# Update the characterization test to reflect the FIX (now 6, not 7):
def test_process_correct_behavior():
assert process([1, 2, 3]) == 6
```
## Guardrails
- **NEVER "improve" legacy code while changing behavior.** Two goals, two commits.
- **Don't fix bugs you weren't asked to fix.** You'll break something that depended on the "buggy" behavior.
- **Characterization tests document reality, not ideals.** Don't "fix" the assertions to what they *should* be — you'd lose the safety net.
- **Don't rewrite without a seam.** A full rewrite of untested code is how you lose weeks and reintroduce old bugs.
- **Make the seam BEFORE the change**, in a separate commit, so the behavior-preserving refactor is provable.
## Pitfalls
| Pitfall | Fix |
|---------|-----|
| "Improving" code while changing behavior accidentally | Characterization tests first; separate behavior from structure |
| Fixing bugs not asked for | Scope the change; ask before fixing unrelated issues |
| Full rewrite instead of surgical change | Slice the change to the exact lines |
| Skipping the seam (hard-coded deps) | Create a seam (default param) before touching the logic |
| Treating characterization tests as "correct" | They pin reality; don't confuse with spec tests |
## Verify / Checklist
- [ ] Characterization tests pin current behavior before editing
- [ ] Seam identified or created (dependency injectable)
- [ ] Change is minimal and localized (a few lines, not a rewrite)
- [ ] Characterization test updated to reflect intentional change only
- [ ] Full suite passes after the change
- [ ] No unrelated "improvements" snuck in (diff reviewed)
Attached files
No attached files.