Refactor Safely

A refactor changes the shape of code and nothing else. Done well it is a series of small, boring, verified steps, each one committed, any one of which can be reverted alone. Done badly it is a big-bang rewrite that subtly changes behaviour and cannot be reviewed. The contract: same inputs, same outputs, same side effects, before and after, proven by tests.

When to use

• "Clean this up", "simplify this function", "split this 3,000-line file", "remove the duplication between these two modules".
• Preparing code so a new feature fits ("make the change easy, then make the easy change").
• After a feature lands, a cleanup pass over your own diff (duplication, needless complexity, wrong-layer fixes).

What you need

• The goal in one sentence (what will be better: readability, a seam for a feature, smaller modules) and the scope (which files or modules).
• The project's test, lint and type-check commands and a green baseline. Refactoring on a red suite means you cannot tell what you broke.
• Write access via the GitHub or GitLab connected app or a git credential, on a branch of its own.

Method

1. State the no-behaviour-change contract and the scope. Write down what must stay identical: return values, exceptions, side effects, public names, log lines other systems parse, performance characteristics that matter. Anything you want to change in behaviour goes in a separate change.

2. Build the safety net. Run the tests that cover the code. If coverage is thin, add characterisation tests first: call the code with representative inputs (including weird ones) and assert on whatever it returns today, even if it looks wrong. For complex output, a golden-file test (save the output once, compare later) is fine as a temporary net. Note odd behaviour you found as a separate issue rather than fixing it now.

3. Choose the moves. Plan a sequence of small, named refactorings. The common ones, with their mechanics, are in references/refactoring-moves.md:

   • extract function or module; inline a needless wrapper;
   • rename (symbol and every reference, including strings);
   • move code between modules, leaving a re-export when the old path is public;
   • replace nested conditionals with guard clauses; replace a long if/else ladder with a lookup table;
   • introduce a parameter object for long argument lists;
   • consolidate duplicated logic (once it appears three times, or twice with a bug fixed in only one copy);
   • split a phase (parse, then compute, then format) or split a god file by responsibility.

4. Look for these simplification targets in the area, and fix the ones in scope:

   • Reuse: hand-rolled logic that an existing helper already does; search_files for similar function names and patterns before writing new helpers.
   • Redundant state: values stored that can be derived; caches that are never invalidated or never needed.
   • Parameter sprawl: flags and options bolted on until the function does several jobs.
   • Deep nesting: three or more levels of if/else, nested ternaries.
   • Stringly typed code: raw strings where an enum, constant or registry already exists.
   • Wrong altitude: a special case for one caller patched into a shared path, or the same workaround repeated at several call sites when the shared function should be fixed. Flag these; fixing the shared path may be its own change.
   • Noise: comments that restate the code, defensive checks on values already validated, commented-out code.

5. Check why before you remove (Chesterton's fence). Before deleting a check, a special case or an odd-looking line, run git log -L <start>,<end>:<file> or git blame and read the commit that added it. If you cannot find the reason, keep it and note it, or ask.

6. Use the tools that see structure.

   • The type checker or compiler is your best safety net: after each step, tsc --noEmit, mypy, cargo check, go build ./... list every broken reference.
   • Language-aware renames where available (the language server's rename, gopls rename, IDE-grade tools the project already uses).
   • Structural search and replace with ast-grep for repetitive edits:

     ast-grep run --pattern 'oldName($$$ARGS)' --rewrite 'newName($$$ARGS)' --lang ts src/    # preview
     ast-grep run --pattern 'oldName($$$ARGS)' --rewrite 'newName($$$ARGS)' --lang ts -U src/ # apply
     

   • search_files for string references the compiler cannot see: reflection, getattr, dependency-injection names, templates, routes, serialisers, config files, docs, other repositories.

7. One step at a time, green after each. For each move: read the files, make the change with patch, run the type checker and the covering tests, then commit with a refactor: message. If a step goes red, revert that step (git restore <files> or git checkout -- <files>) and take a smaller one. Do not debug a broken refactor on top of more edits.

8. Protect public contracts. Exported names, package entry points, API routes and payload fields, database columns, config keys, CLI flags, event names and anything other repos import are contracts. Renaming or moving them needs a compatibility shim (alias, re-export, old route forwarding) and a deprecation note, or the owner's explicit agreement to break them.

9. Keep behaviour fixes out. If you find a bug mid-refactor, write it down, finish or park the refactor, then fix the bug in its own commit or change with its own test. Mixed commits make reviews and reverts painful.

10. Verify the whole. Full test suite, linter, formatter check, type checker and build, compared with the baseline. Review the full diff: it should contain only structural changes. If the code is on a hot path, run a quick before-and-after benchmark to make sure the refactor did not make it slower.

Output

A branch of small refactor: commits and a note with: the goal, the moves made (one line each), anything flagged but not changed (with reasons), public contracts kept compatible and how, bugs found and logged separately, and the checks run with results. Lines added and removed (git diff --shortstat) are a useful headline. For example:

Goal: split `billing/service.py` (2,100 lines) so invoices and refunds can change independently.
Moves (7 commits, +640 / -910 lines):
- Extracted `billing/invoices.py` and `billing/refunds.py`; `service.py` re-exports the old names.
- Replaced the 14-branch tax ladder with a lookup table (`TAX_RULES`).
- Removed 3 pass-through wrappers; consolidated two copies of `round_money`.
Flagged, not changed: `apply_credit()` special-cases one customer ID (`service.py:812`); likely a band-aid, needs the owner's call.
Bug found and logged: refunds round half-down in one path (#731), behaviour unchanged here.
Checks: characterisation tests added first (12); full suite, mypy and ruff clean after every commit.

Checks before you finish

• Tests covering the refactored code existed or were added before the first change, and pass now.
• Each commit builds and passes the covering tests on its own.
• The diff contains no behaviour changes; any found bugs are logged separately.
• Every reference to renamed or moved code was updated, including string references.
• Public contracts are unchanged or have compatibility shims, or the owner agreed to break them.

Pitfalls

• The big-bang rewrite. Replacing a module in one go is not a refactor; it is a rewrite with a refactor's name and none of its safety.
• Refactoring without tests. Without a net you are just changing code and hoping.
• Fixing bugs along the way. It feels efficient and makes the change impossible to review or revert cleanly.
• Trusting dead-looking code is dead. Dynamic references hide from the compiler.
• Premature abstraction. Merging two things that only look alike couples them forever. Wait for the third copy or a shared reason to change.
• Style churn. Reformatting untouched code inflates the diff and hides the real change.
• Breaking someone else's import. Search other packages and ask about other repos before moving public code.