Refactoring

Refactoring

Improve code structure without changing behavior. One small step at a time.

Iron Law: ONE REFACTORING → TEST → COMMIT. Never batch changes.

When to Use

Answer IN ORDER. Stop at first match:

1. User says "refactor", "clean up", "restructure"? → Use this skill
2. User asks to "extract", "rename", "simplify"? → Use this skill
3. Code smell identified? → Use this skill
4. User wants to add feature or fix bug? → Skip (use tdd-enforcer)
5. User wants formatting/style fixes? → Skip (use /lint)

Code smells (common triggers):

• Duplicated code (same logic in multiple places)
• Long function (>30 lines, doing too much)
• Magic numbers/strings (unexplained literals)
• Deep nesting (>3 levels of indentation)
• Dead code (unused functions, unreachable branches)
• Poor naming (unclear what something does)

Phase 1: ASSESS

Is this actually refactoring?

If not refactoring: Explain and suggest correct approach.

Phase 2: PROTECT

Does the code have tests?

Characterization Tests

Capture current behavior before refactoring:

// Characterization test - captures ACTUAL behavior
it('processOrder returns current behavior', () => {
  const result = processOrder({ items: [], user: null });
  // Whatever it returns NOW is the expected value
  expect(result).toEqual({ status: 'empty', total: 0 });
});

Purpose: Safety net, not specification. Test what the code DOES, not what it SHOULD do.

Phase 3: REFACTOR

Iron Law: ONE refactoring at a time. Run tests after EVERY change.

Refactoring Catalog

Tier 1 - Always Safe (no behavior change possible):

// ❌ Before: unclear name
const d = price * 0.2;

// ✅ After: Rename
const discountAmount = price * 0.2;

Tier 2 - Safe with Tests (low risk if tests exist):

// ❌ Before: nested conditionals
function getDiscount(user) {
  if (user) {
    if (user.isVIP) {
      return 0.2;
    } else {
      return 0.1;
    }
  }
  return 0;
}

// ✅ After: Guard Clauses
function getDiscount(user) {
  if (!user) return 0;
  if (user.isVIP) return 0.2;
  return 0.1;
}

Tier 3 - Requires Care (higher risk, break into smaller steps):

Tie-breaker: If multiple refactorings apply, choose smallest scope first (Rename < Extract Variable < Extract Function < Extract Class).

Phase 4: VERIFY

After each refactoring:

1. Run tests - Must pass
2. If tests pass: Commit with refactor: [what changed]
3. If tests fail: Revert immediately

Revert Protocol

git checkout -- <changed-files>

After revert:

• Was the refactoring too large? → Try smaller step
• Did it accidentally change behavior? → Reconsider approach
• DO NOT attempt to "fix" a failed refactoring

After 2 Failed Attempts

STOP. Ask user:

"I've attempted this refactoring twice and tests keep failing. This suggests either:

1. The refactoring is too large (need smaller steps)
2. The code has hidden dependencies
3. Tests are brittle

How would you like to proceed?"

Phase 5: ITERATE

More refactoring needed?
├─ Yes → Return to Phase 3 (one more refactoring)
└─ No → Done
    └─ Report: "Refactoring complete. Changes: [summary]"

Edge Cases

Partial test coverage:

• Identify which functions are tested vs untested
• Add characterization tests only for code you're about to refactor
• Don't boil the ocean - test what you touch

Refactoring reveals a bug:

• STOP refactoring
• Note the bug location
• Ask user: "Found potential bug at X. Fix it now (switching to tdd-enforcer) or continue refactoring?"

User requests large refactoring:

• Break into steps: "I'll refactor this incrementally. First: [step 1]"
• Complete each step fully before next
• Never batch multiple refactorings in one edit

Anti-Patterns

Key Takeaways

1. One change at a time - Never batch refactorings
2. Tests before refactoring - No safety net = no refactoring
3. Revert on failure - Don't fix, revert and retry smaller
4. Commit after each success - refactor: [description]
5. Smallest scope first - Rename < Extract < Move < Restructure