Refactor master

refactor-master — change shape without changing behavior

When to use this skill

Trigger when the user wants the code reorganized but expects the same outputs. Strong signals:

• "refactor this", "clean up", "split", "decompose", "extract"
• "this file is 1200 lines, help"
• "make this readable"
• "pull the logic out of the React component"

Do not trigger for: behavior changes (that's a feature, not a refactor), performance work (use perf-hunter), or a one-line rename (just do it).

The output contract

Refactors that:

1. Preserve behavior — every existing test still passes. If there are no tests, add a characterization test first (see step 2).
2. Land in small steps — each commit is independently reviewable and reverts cleanly.
3. Reduce one specific kind of complexity — say which one upfront (cyclomatic, depth, length, coupling, naming) and measure before/after.
4. Don't leak new abstractions — no BaseFooFactoryProviderInterface unless there are at least three concrete implementations that need it.

Workflow

1 — Diagnose

Read the target. Name the specific smell, out loud, before touching:

• Length: function/file is too long → extract by responsibility
• Depth: too many levels of nesting → early returns, guard clauses, or extract inner blocks
• Repetition: same pattern 3+ times → extract a helper
• God object: one class doing too many things → split by axis of change
• Tangled side effects: pure logic interwoven with I/O → pull out a pure core, push I/O to the edges
• Conditional explosion: switch on a type field with N branches → polymorphism, strategy map, or discriminated union
• Bad names: function names that lie → rename before splitting, never after

If you can't name the smell, the file probably doesn't need refactoring. Stop.

2 — Safety net first

Before changing anything:

• Run the existing tests. Note which pass.
• If coverage is thin, write a characterization test — one test that pins the current behavior for the path you're about to change. It doesn't need to be pretty, just present.
• For UI components, take a screenshot via the preview tools as a visual baseline.

3 — Move in slices

Pick one smell. Address it in one commit. Don't combine refactors.

Common moves, in order of preference:

Extract function — when a block has a clear name and 3+ lines.

// before
if (user.subscription && user.subscription.status === 'active' && user.subscription.plan !== 'trial') { ... }

// after
if (isOnPaidPlan(user)) { ... }

Early return — when nesting is hiding the happy path.

// before
if (a) { if (b) { if (c) { doIt() } } }

// after
if (!a) return
if (!b) return
if (!c) return
doIt()

Extract module — when a file mixes two unrelated concerns. Pull one concern into its own file with a tight export surface.

Pure core / impure shell — when business logic is tangled with database/HTTP/file calls. Move the pure logic into a function that takes plain data and returns plain data. The shell stays thin.

Replace conditional with table — when a switch or if/else if chain dispatches on a value.

// before
switch (kind) {
  case 'image': return processImage(x)
  case 'video': return processVideo(x)
  ...
}

// after
const processors = { image: processImage, video: processVideo, ... }
return processors[kind](x)

Introduce parameter object — when a function takes 5+ args and several are usually passed together.

4 — Verify after every slice

After each commit:

• Run the test suite. If anything fails that wasn't failing before, the refactor changed behavior. Revert and try again.
• For UI: re-screenshot, diff against the baseline.

5 — Stop on time

A refactor session has diminishing returns. After ~3 slices on the same file, ship what you have. Future-you can do more later.

Patterns and anti-patterns

✅ Do:

• Refactor right before adding a feature that touches the messy area. The new feature pays for the cleanup.
• Use git mv (not delete + create) when moving files, so blame survives.
• Keep public APIs stable. Refactor the internals, leave the import path alone.
• Delete dead code aggressively. Git remembers it if you need it back.

❌ Don't:

• Don't refactor without tests. You'll change behavior and not know it.
• Don't combine a refactor and a feature in one PR. The reviewer can't tell what's behavioral.
• Don't introduce DI containers, plugin systems, or abstract base classes to "future-proof" a 200-line file. YAGNI.
• Don't rename a public function without greping every caller, including string-based ones (DI tokens, dynamic imports).

Example invocation

User: "src/checkout.ts is 800 lines and I can't find anything. Help."

1. Diagnose: god module. Mixes pricing logic, tax calculation, payment provider calls, email sending, and DB writes.
2. Safety net: pricing + tax have unit tests; payment + email do not. Write 2 characterization tests covering the happy-path checkout and one error path.
3. Slice 1: extract pricing.ts (pure functions only). Commit.
4. Slice 2: extract tax.ts (pure). Commit.
5. Slice 3: extract paymentProvider.ts (the Stripe-specific code). Commit.
6. Now checkout.ts is ~300 lines and orchestrates the four pieces. Run tests — all green.
7. Stop. Ship. Note: notifications and persistence are still tangled in there; a follow-up refactor can split those.

See also

• test-architect — to add the characterization tests safely
• code-auditor — to find the smells worth refactoring
• perf-hunter — when the refactor is motivated by performance, not readability