Refactoring
Purpose
Change the structure of code while provably preserving behavior. The defining constraint is that behavior does not change — if it does, it is a rewrite, and it needs different scrutiny.
When to Use
- Before adding a feature to code that resists the change.
- When a module has become hard to test.
- To pay down a specific, identified cost — not to satisfy an aesthetic.
- After a bug reveals a structural cause.
Capabilities
- Characterization tests to pin down existing behavior, including its bugs.
- Standard sequences: extract function, extract class, inline, replace conditional with polymorphism, introduce parameter object.
- Dependency breaking: seams, adapters, and inversion for untestable code.
- Incremental strangling of legacy paths behind a stable interface.
Inputs
- The module to change and the reason for changing it.
- The existing test suite, and an honest assessment of its coverage.
- The feature or fix that motivated the refactor, if any.
Outputs
- Restructured code with identical observable behavior.
- Tests that passed before and after, unchanged.
- A commit sequence where each commit is independently green.
Workflow
- Justify it — Name the concrete cost the current structure imposes. "It's ugly" is not a cost. "Every new payment method requires editing five files" is.
- Pin the behavior — If tests do not cover the code, write characterization tests first: call the code, record what it actually does, assert that. Bugs included.
- Refactor in small steps — One mechanical transformation per commit. Run the tests after each.
- Do not mix in behavior changes — A refactoring commit and a feature commit must be separate. Reviewers cannot verify both at once.
- Stop when the motivating change is easy — Refactoring is preparation, not a destination.
Best Practices
- Make the change easy, then make the easy change. In that order, in separate commits.
- Never refactor on a branch that is not fully covered by tests you trust.
- If the tests need to change, you are not refactoring. Say so.
- Extract until each function does one thing at one level of abstraction, and no further.
- Delete dead code aggressively; version control remembers it for you.
- Resist wholesale rewrites. The existing code encodes years of edge cases you have not thought of.
Examples
Breaking a dependency to make code testable:
# Before: untestable — reaches out to the network and the clock.
def send_expiry_warnings():
for user in db.query("SELECT * FROM users WHERE expires_at < now() + interval '7 days'"):
smtp.send(user.email, render("expiry_warning", user=user))
# After: dependencies are seams. Behavior is unchanged.
def find_expiring_users(db, now, window=timedelta(days=7)):
return db.users_expiring_before(now + window)
def send_expiry_warnings(db, mailer, now):
for user in find_expiring_users(db, now):
mailer.send(user.email, render("expiry_warning", user=user))
The test can now supply a fake db, a fake mailer, and a fixed now — without a network, a database, or a sleep.
Notes
- Characterization tests will encode existing bugs. That is intentional: fix them in a separate, clearly labeled commit so the fix is reviewable.
- Automated refactorings in an IDE (rename, extract method) are safer than hand edits. Prefer them.
- A refactor that touches a hundred files is not small steps, whatever the commit message says. Split by module.
1---2name: refactoring3description: Use when improving the structure of code without changing its behavior. Covers safe refactoring sequences, characterization tests, and knowing when to stop.4---56# Refactoring78## Purpose910Change the structure of code while provably preserving behavior. The defining constraint is that behavior does not change — if it does, it is a rewrite, and it needs different scrutiny.1112## When to Use1314- Before adding a feature to code that resists the change.15- When a module has become hard to test.16- To pay down a specific, identified cost — not to satisfy an aesthetic.17- After a bug reveals a structural cause.1819## Capabilities2021- Characterization tests to pin down existing behavior, including its bugs.22- Standard sequences: extract function, extract class, inline, replace conditional with polymorphism, introduce parameter object.23- Dependency breaking: seams, adapters, and inversion for untestable code.24- Incremental strangling of legacy paths behind a stable interface.2526## Inputs2728- The module to change and the reason for changing it.29- The existing test suite, and an honest assessment of its coverage.30- The feature or fix that motivated the refactor, if any.3132## Outputs3334- Restructured code with identical observable behavior.35- Tests that passed before and after, unchanged.36- A commit sequence where each commit is independently green.3738## Workflow39401. **Justify it** — Name the concrete cost the current structure imposes. "It's ugly" is not a cost. "Every new payment method requires editing five files" is.412. **Pin the behavior** — If tests do not cover the code, write characterization tests first: call the code, record what it actually does, assert that. Bugs included.423. **Refactor in small steps** — One mechanical transformation per commit. Run the tests after each.434. **Do not mix in behavior changes** — A refactoring commit and a feature commit must be separate. Reviewers cannot verify both at once.445. **Stop when the motivating change is easy** — Refactoring is preparation, not a destination.4546## Best Practices4748- Make the change easy, then make the easy change. In that order, in separate commits.49- Never refactor on a branch that is not fully covered by tests you trust.50- If the tests need to change, you are not refactoring. Say so.51- Extract until each function does one thing at one level of abstraction, and no further.52- Delete dead code aggressively; version control remembers it for you.53- Resist wholesale rewrites. The existing code encodes years of edge cases you have not thought of.5455## Examples5657**Breaking a dependency to make code testable:**5859```python60# Before: untestable — reaches out to the network and the clock.61def send_expiry_warnings():62 for user in db.query("SELECT * FROM users WHERE expires_at < now() + interval '7 days'"):63 smtp.send(user.email, render("expiry_warning", user=user))6465# After: dependencies are seams. Behavior is unchanged.66def find_expiring_users(db, now, window=timedelta(days=7)):67 return db.users_expiring_before(now + window)6869def send_expiry_warnings(db, mailer, now):70 for user in find_expiring_users(db, now):71 mailer.send(user.email, render("expiry_warning", user=user))72```7374The test can now supply a fake `db`, a fake `mailer`, and a fixed `now` — without a network, a database, or a sleep.7576## Notes7778- Characterization tests will encode existing bugs. That is intentional: fix them in a separate, clearly labeled commit so the fix is reviewable.79- Automated refactorings in an IDE (rename, extract method) are safer than hand edits. Prefer them.80- A refactor that touches a hundred files is not small steps, whatever the commit message says. Split by module.