refactoring-patterns

# Refactoring Patterns Skill

## When to Use

Invoke this skill when:
- Code smells are detected (long functions, duplicated logic, shotgun surgery)
- Before making major structural changes to existing code
- During tech debt sprints or cleanup tasks
- A function exceeds 30 lines and needs decomposition
- Renaming, moving, or reorganizing code across files or packages

## The Golden Rule

```
Tests green -> Refactor -> Tests green
```

Never refactor code that lacks test coverage. If tests don't exist, write them first (characterization tests that capture current behavior), then refactor.

## Safe Refactoring Steps

1. **Verify tests pass** before touching anything. Run the full suite for the affected area.
2. **Make one refactoring move at a time.** Never combine multiple refactorings in a single change.
3. **Run tests after each move.** If anything fails, you know exactly which move broke it.
4. **Commit after each successful step.** Small commits make `git bisect` possible and rollback trivial.
5. **If tests fail, revert the last move.** Do not debug forward -- undo and try a different approach.

```
[tests pass] -> extract method -> [tests pass] -> commit
             -> rename variable -> [tests pass] -> commit
             -> move to new file -> [tests pass] -> commit
             -> [tests FAIL] -> git revert -> rethink
```

## Common Patterns

### Extract Method

**Smell:** Function too long, or comments explaining what a block does.

**Action:** Pull the block into a named function. The function name replaces the comment.

**When:** A function has distinct logical sections separated by blank lines or comments. Each section becomes its own function.

### Extract Class / Module

**Smell:** A class or module has multiple unrelated responsibilities. Changes for different reasons.

**Action:** Split into separate classes/modules, each with a single responsibility.

**When:** You find yourself prefixing method groups (`userCreate`, `userDelete`, `orderCreate`, `orderDelete` in one class) or the file exceeds 500 lines with distinct sections.

### Inline

**Smell:** Indirection adds complexity without clarity. A function just calls another function. A variable is used once and the expression is self-explanatory.

**Action:** Replace the indirection with the direct call or expression.

**When:** The abstraction layer adds no value -- the delegated function is equally readable inline.

### Move

**Smell:** A function or type is referenced more from another package/module than from its current one.

**Action:** Move it to where it belongs. Update all references.

**When:** Feature envy -- a function uses more data from another module than its own. Or the current package has grown a "utils" section that really belongs elsewhere.

### Rename

**Smell:** Name doesn't reveal intent. Requires reading the implementation to understand purpose.

**Action:** Rename to communicate what it does, not how it does it.

**When:** You find yourself re-reading a function body to remember what it does. Variable names like `data`, `result`, `temp`, `val`, or single letters outside tight loops.

### Replace Conditional with Polymorphism

**Smell:** Switch/if chains that check a type field and execute different logic per type.

**Action:** Define an interface with the varying behavior. Each type implements the interface.

**When:** The same type-check conditional appears in multiple places. Adding a new type requires modifying multiple switch statements.

### Introduce Parameter Object

**Smell:** Three or more related parameters always travel together across multiple functions.

**Action:** Group them into a struct, class, or typed object.

**When:** You see the same cluster of parameters in multiple function signatures (`startDate, endDate, timezone` or `host, port, protocol`).

### Replace Magic Number with Named Constant

**Smell:** Literal values embedded in logic with no explanation of their meaning.

**Action:** Extract to a named constant that explains the value's purpose.

**When:** A number or string literal appears in a condition, calculation, or configuration and its meaning is not immediately obvious (`if retries > 3`, `timeout: 30000`).

## Per-Language Tooling

| Language | Rename | Extract / Refactor | Format / Fix |
|----------|--------|---------------------|--------------|
| Go | `gorename`, `gopls rename` | `gopls refactor.extract` | `go fmt`, `goimports` |
| TypeScript | `ts-morph`, IDE rename symbol | IDE extract function/variable | `eslint --fix`, `prettier` |
| Python | `rope`, IDE rename symbol | IDE extract method/variable | `ruff --fix`, `black` |

Prefer IDE-assisted refactoring (rename symbol, extract function) over manual find-and-replace. Automated tools update all references and catch type errors.

## Red Flags -- When NOT to Refactor

- **No tests cover the code being refactored.** Write characterization tests first or accept the risk explicitly.
- **Refactoring across module boundaries without coordination.** If the public API changes, downstream consumers break. Coordinate or version the API.
- **Changing public APIs without versioning.** If external code depends on it, add a new version instead of modifying in place.
- **Mixing refactoring with feature work in the same commit.** Refactoring commits should be behavior-preserving. Feature commits should add new behavior. Mixing makes rollback impossible and code review painful.
- **Refactoring code you don't understand.** Read it first. Write characterization tests. Talk to the original author if available. Refactoring without understanding introduces subtle bugs.

## Pre-Ship Checklist

Before merging refactored code, verify:

1. [ ] All tests pass (unit, integration, E2E for affected area)
2. [ ] No behavior changed -- refactoring is strictly structural
3. [ ] Each commit is a single refactoring move (reviewable in isolation)
4. [ ] No refactoring and feature work mixed in the same commit
5. [ ] Public API signatures are unchanged (or changes are versioned and documented)
6. [ ] No dead code left behind (or marked with `// TODO: AI_DELETION_REVIEW`)
7. [ ] Code review confirms readability improved (the whole point of refactoring)
8. [ ] CI pipeline is green on the final state