refactoring

name

refactoring

description

Use for code-quality refactoring tasks: extract function from a long method, rename a confusingly-named symbol, restructure a tangled module, replace a conditional dispatch with polymorphism, split a god-class along its responsibilities, and introduce dependency injection so I/O can be faked in tests. Behavior must be preserved; tests stay green; public API stays intact unless the task says otherwise.

Refactoring Investigation Playbook

A refactoring task starts from working-but-ugly code: passing tests, but a smell that makes future change risky. The output is a structurally cleaner version of the same module with the same observable behavior. Move through five phases in order — skipping a phase is the most common cause of refactors that silently change behavior or break the public API.

This skill is about emitting refactored code directly. It does not call out to AST tools (rope, libcst); the agent reads the source, decides the transformation, and writes the new code.

Phase 1 — Detect smells

Read the target code and check it against this list. More than one smell usually fires at once; record all of them before choosing a remedy.

Check	Smell name	What to look for
Size	Long method	A function over ~30 lines or with 3+ levels of nesting
Size	God class	One class doing parsing, business logic, persistence, and formatting
Coupling	Long parameter list	5+ positional parameters, especially repeated across calls
Coupling	Feature envy	Method that mostly reads/writes another object's fields
Cohesion	Divergent change	One module is edited for many unrelated reasons
Cohesion	Shotgun surgery	One conceptual change requires edits in many modules
Duplication	Duplicated logic	Two near-identical blocks differing only in a literal or a callee
Naming	Primitive obsession	Bare str/int for things with invariants (money, ids, units)
Conditional	Tangled conditional	if/elif chain switching on a type tag or string discriminator
I/O entanglement	Untestable unit	Business logic that opens files, hits a DB, or imports a network client inline

A smell is a hypothesis, not a verdict. Confirm it by trying to describe the code in one sentence — if the sentence has "and" in it, the unit is doing more than one thing.

Phase 2 — Diagnose: smell to cause

Map each detected smell to its likely cause. The cause determines which transformation actually helps; the smell alone does not.

Smell	Likely cause	Wrong fix to avoid
Long method	Multiple steps inlined; loop body is a function in disguise	Splitting on line count alone, producing arbitrary halves
God class	Responsibilities accreted over time; no clear domain boundary	Renaming methods without moving them
Long parameter list	Several parameters always travel together (a missing object)	Defaulting parameters to hide them
Feature envy	Behavior lives in the wrong class	Adding a getter and keeping the logic where it is
Duplicated logic	Same intent, copy-pasted; or two intents that look alike	Extracting too early — see Rule of Three
Tangled conditional	Type-tag dispatch where each branch encodes one variant's behavior	Adding more branches
Primitive obsession	A domain concept never got its own type	Validating in every caller
Untestable unit	I/O is constructed inside the function, not passed in	Monkey-patching globals in tests

If two smells point to incompatible transformations, fix the structural one first (god class, untestable unit) before the local one (long method, naming).

Phase 3 — Choose transformation

Pick a transformation from the Fowler catalog that matches the diagnosed cause. Prefer the smallest transformation that removes the smell.

Cause	Transformation	One-line description
Long method with clear sub-step	Extract Method	Pull the sub-step into a named function
Repeated parameter group	Introduce Parameter Object	Wrap the group in a small dataclass
Method belongs elsewhere	Move Method	Relocate it to the class whose data it uses
Type-tag dispatch	Replace Conditional with Polymorphism	One subclass / strategy per branch
Confusing name	Rename Symbol	Choose a name that describes the role, not the type
God class	Extract Class	Split along one responsibility at a time
Rigid base class with sharable parts	Replace Inheritance with Composition	Inject the collaborator instead of extending
Hard-wired I/O	Introduce Dependency Injection	Pass the collaborator (db, clock, http) into __init__
Bare primitive with invariants	Introduce Value Object	Wrap the primitive in a typed class with validation

Rule of Three: do not extract a shared abstraction from two call sites unless the abstraction is already known from elsewhere. Two near-identical blocks often diverge later for unrelated reasons.

Phase 4 — Apply

Implement the chosen transformation as a literal source rewrite. Keep the public API stable: same module-level names, same function signatures (unless the task explicitly authorizes a signature change).

Worked example: Replace Conditional with Polymorphism + Introduce Parameter Object on a tangled pricing function.

# Before: tangled conditional + long parameter list
def price(kind: str, base: float, qty: int, tier: str, country: str) -> float:
    if kind == "book":
        d = 0.10 if tier == "gold" else 0.0
        return base * qty * (1 - d) * (1.0 if country == "US" else 1.2)
    elif kind == "ebook":
        return base * qty * (0.85 if tier == "gold" else 0.95)
    elif kind == "audio":
        return base * qty * (0.80 if tier == "gold" else 1.0) + 1.50 * qty
    raise ValueError(kind)

# After: parameter object + per-kind strategy
from dataclasses import dataclass

@dataclass(frozen=True)
class Order:
    base: float
    qty: int
    tier: str       # "gold" | "standard"
    country: str    # ISO-2

class Pricer:
    def total(self, o: Order) -> float: raise NotImplementedError

class BookPricer(Pricer):
    def total(self, o: Order) -> float:
        d = 0.10 if o.tier == "gold" else 0.0
        intl = 1.0 if o.country == "US" else 1.2
        return o.base * o.qty * (1 - d) * intl

class EbookPricer(Pricer):
    def total(self, o: Order) -> float:
        return o.base * o.qty * (0.85 if o.tier == "gold" else 0.95)

class AudioPricer(Pricer):
    def total(self, o: Order) -> float:
        rate = 0.80 if o.tier == "gold" else 1.0
        return o.base * o.qty * rate + 1.50 * o.qty

PRICERS: dict[str, Pricer] = {"book": BookPricer(), "ebook": EbookPricer(), "audio": AudioPricer()}

def price(kind: str, base: float, qty: int, tier: str, country: str) -> float:
    try:
        return PRICERS[kind].total(Order(base, qty, tier, country))
    except KeyError:
        raise ValueError(kind)

The signature of price is unchanged, so existing callers and tests do not move. New kind values now require adding a class, not editing a chain. The Order dataclass is the parameter object that absorbs the five-arg list at the strategy boundary.

For the other transformations:

• Extract Method. Identify a contiguous block with a single purpose, give it a verb-phrase name, replace the block with a call. Locals used after the block become return values; locals consumed only inside become arguments.
• Move Method. When method A.foo() mostly touches b: B, move foo to B and have A.foo either delegate or be deleted in favor of direct b.foo() calls.
• Extract Class. Pick one responsibility (e.g., serialization) of a god class, list the fields and methods it owns, copy them to a new class, leave a thin delegation in the original until callers migrate.
• Introduce DI. Replace self._db = connect(...) in __init__ with def __init__(self, db: DB): self._db = db. Provide a factory at the composition root for production wiring.
• Replace Inheritance with Composition. Replace class X(Base) reuse with class X: def __init__(self, helper: Base): self._h = helper, forwarding only what is actually used.

Phase 5 — Verify

Confirm structure changed and behavior did not.

• Run the existing test suite. Every test that passed before must still pass; do not edit tests to accommodate the refactor unless they assert on a private name that the task explicitly allows changing.
• Diff the public surface: python -c "import mod; print(sorted(n for n in dir(mod) if not n.startswith('_')))" before and after. New names are fine; removed or renamed public names are a breaking change and must be deliberate.
• Check signatures of any retained public function with inspect.signature — argument names, defaults, and order are part of the contract for keyword callers.
• Sanity-run a representative input through both old and new code (keep the old one in a scratch file during the edit) and assert equality on the result. For the pricing example: a small parametric sweep over (kind, tier, country) is enough.
• Re-read the diff. Drive-by edits (formatting unrelated lines, fixing other bugs, deleting unused imports outside the refactor zone) belong in a separate change.

Pitfalls

• Extracting a function whose name is just a paraphrase of its body (process_item, do_step). The name must describe the purpose, not the mechanics.
• Replacing a 3-branch conditional with 3 classes plus a registry — net more code, no payoff. Polymorphism earns its keep when branches grow or each branch holds non-trivial state.
• Introducing a parameter object that is just a tuple with extra steps; if the wrapper has no behavior and no validation, inline-keyword-args may be cleaner.
• Pulling I/O out of one function but constructing the I/O object inside the caller — DI moved up one frame, not actually injected.
• Renaming a public symbol without updating the import sites the task is graded on. A rename is a refactor only if every reference moves with it.
• Tests still green because they never exercised the changed branch. If coverage of the refactored code is unknown, add a minimal test before refactoring, not after.