Refactoring
Intro
Refactoring is changing the structure of code without changing its
observable behavior. Do it in small, test-protected steps, one
transformation at a time, and never mix it with feature changes in
the same commit.
Overview
Verify the safety net first
Before any refactoring, run the existing tests and confirm they
pass. If no tests cover the code you are about to change, write
characterization tests first — tests that capture current behavior,
not desired behavior. Plan the refactoring as a sequence of atomic
transformations where each step keeps the suite green, and commit
after each step. The golden rule: never mix refactoring with feature
changes in the same commit.
Identify code smells
Scan systematically for the well-known smells. The full catalog
lives in references/code-smells.md; the most common offenders:
| Smell |
What to look for |
| Long Method |
Function > 20 lines or doing more than one thing |
| Large Class |
Class with > 5 responsibilities or > 300 lines |
| Long Parameter List |
Function with > 3 parameters |
| Feature Envy |
Method uses another object's data more than its own |
| Data Clumps |
Same group of variables appears together repeatedly |
| Primitive Obsession |
Strings/ints where a domain type belongs |
| Shotgun Surgery |
One change requires edits in many unrelated files |
| Divergent Change |
One class changed for multiple unrelated reasons |
| Duplicate Code |
Same logic in two or more places |
| Dead Code |
Unreachable or unused code |
Apply Fowler's refactoring catalog
Match smells to specific refactorings:
- Composing methods — Extract Function, Inline Function, Extract
Variable, Replace Temp with Query.
- Moving features — Move Function/Field, Extract Class, Inline
Class.
- Simplifying conditionals — Replace Nested Conditional with
Guard Clauses, Decompose Conditional, Replace Conditional with
Polymorphism, Introduce Null Object.
- Organizing data — Introduce Parameter Object, Replace Magic
Number with Constant, Encapsulate Collection.
- Generalization — Pull Up / Push Down Method, Extract
Interface/Trait, Replace Inheritance with Composition.
Apply GoF patterns when they simplify
Only introduce a design pattern when it solves a concrete problem,
never speculatively. The full catalog is in
references/gof-patterns.md. Most commonly useful: Strategy,
Observer, Factory Method, Builder, Adapter, Decorator, Command,
State, Template Method, Iterator. Warning signs of overuse: adding
a pattern "just in case", pattern adds more code than it removes,
only one implementation exists, the team cannot explain why.
Example
User says "this function is too long". The agent identifies three
distinct responsibilities, extracts each into a named helper
keeping the original as a high-level orchestrator, runs tests after
each extraction, and commits each extraction separately.
Gotchas
Agent-specific failure modes — provider-neutral pause-and-self-check items:
- Mixing refactoring with feature changes in the same commit. When a commit changes both structure and behavior, reviewers cannot verify that behavior was preserved, and rollback becomes surgical. Keep refactoring commits separate; the commit message should start with
refactor:.
- Starting without a test safety net. If the code has no tests, changing its structure with no verification is guessing. Write characterization tests first — tests that capture current behavior, not desired behavior — then refactor.
- Big-bang rewrite instead of incremental transformation. Full rewrites take longer, introduce more bugs, and stall new features during the rewrite. Refactor incrementally: one smell, one transformation, one commit, tests green throughout.
- Removing code assumed to be dead without verifying. Dynamic dispatch, reflection, plugin systems, and config-driven loads can call code that static analysis says is unreachable. Search for all invocation paths before deleting anything.
- Speculative generality. Adding extension points, plugin hooks, or configuration flags "just in case" is not refactoring — it is adding untested code for hypothetical future requirements. Refactor to the current need, not the imagined future.
- Pattern cargo-culting. Introducing a design pattern because it sounds clever adds indirection and complexity without value. Apply a pattern only when it solves a concrete, present problem — and only after the "rule of three" (three similar occurrences) is met.
- Skipping the refactor step after green. The refactor step is where the design payoff from TDD-style cycles lives. Skipping it means the test suite is green but the technical debt is accumulating.
Full reference
Modern considerations
For async/concurrent code, extract async boundaries clearly (sync
core, async shell), replace callback chains with async/await where
possible, and isolate side effects at the edges. For
functional-style code, replace mutable loops with map/filter/reduce
pipelines, extract pure functions from impure ones, and use
algebraic types (enums/unions) over class hierarchies for closed
sets.
Smell-to-pattern map
| Smell / situation |
Candidate pattern |
| Complex conditional logic on types |
Strategy, State, polymorphism |
| Growing switch/match statements |
Factory Method + polymorphism |
| Duplicated algorithm with variations |
Template Method |
| Complex object construction |
Builder |
| Third-party integration coupling |
Adapter |
| Need to add behavior to existing code |
Decorator |
| Multiple objects reacting to events |
Observer |
| Complex state-dependent behavior |
State |
| Request needs undo/queue/log |
Command |
| Many interacting objects |
Mediator |
Selecting a pattern (rule of three)
Wait until you see a need in three places before abstracting. One
occurrence is a fact, two is a coincidence, three is a pattern.
Premature abstraction is harder to undo than late abstraction
because the wrong shape constrains future changes.
Anti-patterns
- Mixing refactoring with feature work in the same commit
- Refactoring without a test safety net
- Speculative generality — adding hooks "just in case"
- Pattern cargo-culting — applying GoF because it sounds clever
- Big-bang rewrites instead of incremental transformation
- Removing dead code without checking it is truly unreachable
(search for dynamic dispatch, reflection, config-driven loads)
Example: extracting test seams
To make code more testable, identify concrete dependencies (file
I/O, HTTP, database), extract interfaces/traits at those
boundaries (Adapter pattern), move domain logic into pure
functions, and introduce dependency injection so tests can supply
fakes. This is refactoring in service of testability, not feature
work, and should ship in its own commit series.
1---2name: refactoring3description: Systematic refactoring with Fowler's catalog, GoF patterns, and code smell detection. Use when restructuring code without changing behavior — cleaning up smells, applying patterns, reducing duplication, or preparing a module for an upcoming feature.4---56# Refactoring78## Intro910Refactoring is changing the structure of code without changing its11observable behavior. Do it in small, test-protected steps, one12transformation at a time, and never mix it with feature changes in13the same commit.1415## Overview1617### Verify the safety net first1819Before any refactoring, run the existing tests and confirm they20pass. If no tests cover the code you are about to change, write21characterization tests first — tests that capture current behavior,22not desired behavior. Plan the refactoring as a sequence of atomic23transformations where each step keeps the suite green, and commit24after each step. The golden rule: never mix refactoring with feature25changes in the same commit.2627### Identify code smells2829Scan systematically for the well-known smells. The full catalog30lives in `references/code-smells.md`; the most common offenders:3132| Smell | What to look for |33|---|---|34| Long Method | Function > 20 lines or doing more than one thing |35| Large Class | Class with > 5 responsibilities or > 300 lines |36| Long Parameter List | Function with > 3 parameters |37| Feature Envy | Method uses another object's data more than its own |38| Data Clumps | Same group of variables appears together repeatedly |39| Primitive Obsession | Strings/ints where a domain type belongs |40| Shotgun Surgery | One change requires edits in many unrelated files |41| Divergent Change | One class changed for multiple unrelated reasons |42| Duplicate Code | Same logic in two or more places |43| Dead Code | Unreachable or unused code |4445### Apply Fowler's refactoring catalog4647Match smells to specific refactorings:4849- **Composing methods** — Extract Function, Inline Function, Extract50 Variable, Replace Temp with Query.51- **Moving features** — Move Function/Field, Extract Class, Inline52 Class.53- **Simplifying conditionals** — Replace Nested Conditional with54 Guard Clauses, Decompose Conditional, Replace Conditional with55 Polymorphism, Introduce Null Object.56- **Organizing data** — Introduce Parameter Object, Replace Magic57 Number with Constant, Encapsulate Collection.58- **Generalization** — Pull Up / Push Down Method, Extract59 Interface/Trait, Replace Inheritance with Composition.6061### Apply GoF patterns when they simplify6263Only introduce a design pattern when it solves a concrete problem,64never speculatively. The full catalog is in65`references/gof-patterns.md`. Most commonly useful: Strategy,66Observer, Factory Method, Builder, Adapter, Decorator, Command,67State, Template Method, Iterator. Warning signs of overuse: adding68a pattern "just in case", pattern adds more code than it removes,69only one implementation exists, the team cannot explain why.7071### Example7273User says "this function is too long". The agent identifies three74distinct responsibilities, extracts each into a named helper75keeping the original as a high-level orchestrator, runs tests after76each extraction, and commits each extraction separately.7778## Gotchas7980Agent-specific failure modes — provider-neutral pause-and-self-check items:8182- **Mixing refactoring with feature changes in the same commit.** When a commit changes both structure and behavior, reviewers cannot verify that behavior was preserved, and rollback becomes surgical. Keep refactoring commits separate; the commit message should start with `refactor:`.83- **Starting without a test safety net.** If the code has no tests, changing its structure with no verification is guessing. Write characterization tests first — tests that capture current behavior, not desired behavior — then refactor.84- **Big-bang rewrite instead of incremental transformation.** Full rewrites take longer, introduce more bugs, and stall new features during the rewrite. Refactor incrementally: one smell, one transformation, one commit, tests green throughout.85- **Removing code assumed to be dead without verifying.** Dynamic dispatch, reflection, plugin systems, and config-driven loads can call code that static analysis says is unreachable. Search for all invocation paths before deleting anything.86- **Speculative generality.** Adding extension points, plugin hooks, or configuration flags "just in case" is not refactoring — it is adding untested code for hypothetical future requirements. Refactor to the current need, not the imagined future.87- **Pattern cargo-culting.** Introducing a design pattern because it sounds clever adds indirection and complexity without value. Apply a pattern only when it solves a concrete, present problem — and only after the "rule of three" (three similar occurrences) is met.88- **Skipping the refactor step after green.** The refactor step is where the design payoff from TDD-style cycles lives. Skipping it means the test suite is green but the technical debt is accumulating.8990## Full reference9192### Modern considerations9394For async/concurrent code, extract async boundaries clearly (sync95core, async shell), replace callback chains with async/await where96possible, and isolate side effects at the edges. For97functional-style code, replace mutable loops with map/filter/reduce98pipelines, extract pure functions from impure ones, and use99algebraic types (enums/unions) over class hierarchies for closed100sets.101102### Smell-to-pattern map103104| Smell / situation | Candidate pattern |105|---|---|106| Complex conditional logic on types | Strategy, State, polymorphism |107| Growing switch/match statements | Factory Method + polymorphism |108| Duplicated algorithm with variations | Template Method |109| Complex object construction | Builder |110| Third-party integration coupling | Adapter |111| Need to add behavior to existing code | Decorator |112| Multiple objects reacting to events | Observer |113| Complex state-dependent behavior | State |114| Request needs undo/queue/log | Command |115| Many interacting objects | Mediator |116117### Selecting a pattern (rule of three)118119Wait until you see a need in three places before abstracting. One120occurrence is a fact, two is a coincidence, three is a pattern.121Premature abstraction is harder to undo than late abstraction122because the wrong shape constrains future changes.123124### Anti-patterns125126- Mixing refactoring with feature work in the same commit127- Refactoring without a test safety net128- Speculative generality — adding hooks "just in case"129- Pattern cargo-culting — applying GoF because it sounds clever130- Big-bang rewrites instead of incremental transformation131- Removing dead code without checking it is truly unreachable132 (search for dynamic dispatch, reflection, config-driven loads)133134### Example: extracting test seams135136To make code more testable, identify concrete dependencies (file137I/O, HTTP, database), extract interfaces/traits at those138boundaries (Adapter pattern), move domain logic into pure139functions, and introduce dependency injection so tests can supply140fakes. This is refactoring in service of testability, not feature141work, and should ship in its own commit series.