third774/refactoring-code

Refactoring Code

You are following a systematic refactoring methodology that improves code quality, maintainability, and simplicity while preserving existing behavior.

Core Principle: Refactoring changes structure, not functionality. Safety and clarity are more important than elegance.

The Iron Law

BEHAVIOR MUST BE PRESERVED
Tests must verify behavior, not implementation

If you change what the code does (not just how it does it), you're not refactoring—you're rewriting.

Prerequisites

• Access to codebase for pattern research
• Behavior-driven tests that verify current functionality
• Understanding of project conventions

Core Responsibilities

1. Simplify Complex Code - Break down complex functions, reduce nesting, improve readability
2. Extract Patterns - Identify and extract reusable components, hooks, utilities
3. Eliminate Duplication - Apply DRY principles while avoiding premature abstraction
4. Improve Type Safety - Strengthen TypeScript usage, eliminate any types when possible
5. Performance Optimization - Identify unnecessary re-renders, optimize data structures
6. Align with Standards - Ensure code follows project patterns and conventions

The Five-Phase Refactoring Process

You MUST complete each phase before proceeding to the next.

Copy this checklist and track your progress:

Refactoring Progress:
- [ ] Phase 1: Understand Current Behavior
  - [ ] Read existing code thoroughly
  - [ ] Check usage across codebase
  - [ ] Document current behavior
- [ ] Phase 2: Verify Test Coverage (CRITICAL)
  - [ ] Behavior-driven tests exist and pass
  - [ ] Tests cover main workflows and edge cases
  - [ ] Tests don't depend on implementation details
- [ ] Phase 3: Identify Issues
  - [ ] Issues documented with locations
  - [ ] Issues categorized by type and severity
  - [ ] Root cause understood for each issue
- [ ] Phase 4: Plan Refactoring
  - [ ] Broken into small, verifiable steps
  - [ ] Each step has verification criteria
  - [ ] Dependencies between steps identified
- [ ] Phase 5: Execute & Verify
  - [ ] All planned changes implemented
  - [ ] All tests pass
  - [ ] No new type errors or warnings
  - [ ] Behavior verified unchanged

Phase 1: Understand Current Behavior

Objective: Know exactly what the code does before changing how it does it.

1. Read existing code thoroughly

   • Understand the purpose, not just the implementation
   • Identify inputs, outputs, and side effects
   • Note any edge cases or error handling

2. Check usage across codebase

   • Use Grep to find all call sites
   • Understand how consumers depend on this code
   • Identify any public APIs that must remain stable

3. Document current behavior

   • List what the code accomplishes
   • Note any implicit contracts or assumptions
   • Identify performance characteristics if relevant

Phase 1 Complete When:

• [ ] You can explain what the code does in plain language
• [ ] All call sites and dependencies are identified
• [ ] Current behavior is documented

Phase 2: Verify Test Coverage (CRITICAL)

Objective: Ensure behavior-driven tests exist that will catch regressions.

BEFORE refactoring, ensure tests verify BEHAVIOR (what users see/do), NOT implementation:

✅ Good - Behavior-driven tests:

// Tests user-observable behavior
test('displays error message when API returns 404', async () => {
  server.use(
    http.get('/api/users', () => new HttpResponse(null, { status: 404 }))
  );
  render(<UserList />);
  expect(await screen.findByText(/user not found/i)).toBeInTheDocument();
});

❌ Bad - Implementation-detail tests:

// Tests internal state/functions - breaks during refactoring
test('sets error state when fetch fails', () => {
  const wrapper = shallow(<UserList />);
  wrapper.instance().handleError(new Error('404'));
  expect(wrapper.state('error')).toBe('404');
});

Test Quality Checklist:

• [ ] Tests verify user workflows, not function calls
• [ ] Tests use real dependencies (not mocks of components)
• [ ] Tests query by role/label/text, not internal state
• [ ] Tests will survive refactoring when behavior unchanged

If tests are missing or test implementation details:

1. Add behavior-driven tests first (see writing-tests skill)
2. Focus on user workflows, loading/error states, interactions
3. Ensure new tests are passing before refactoring

Why this matters: Unit tests checking internal state break during refactoring even when behavior is unchanged. Behavior-driven tests prove the feature works before and after.

Phase 2 Complete When:

• [ ] Behavior-driven tests exist and pass
• [ ] Tests cover main workflows and edge cases
• [ ] Tests don't depend on implementation details

Phase 3: Identify Issues

Objective: Systematically catalog what needs improvement.

Common refactoring opportunities:

| Issue Type | Indicators | Impact | |------------|-----------|---------| | Complexity | Deep nesting (>3 levels), long functions (>50 lines), many parameters (>4) | Hard to understand, error-prone | | Duplication | Copy-pasted code, repeated patterns, similar logic in multiple places | Maintenance burden, inconsistent fixes | | Poor Naming | Unclear variables (x, data, temp), misleading names, inconsistent terminology | Cognitive overhead | | Type Safety Gaps | any types, missing types, type assertions, implicit any | Runtime errors, poor IDE support | | Performance Issues | Unnecessary re-renders, inefficient algorithms (O(n²) when O(n) possible), large bundle size | Slow UX, high resource usage | | Pattern Violations | Inconsistent with project conventions, outdated patterns | Team confusion, tech debt |

Analysis checklist:

• [ ] Identify specific complexity hotspots
• [ ] Catalog duplicated code blocks
• [ ] List unclear or misleading names
• [ ] Find type safety vulnerabilities
• [ ] Measure performance bottlenecks (if applicable)
• [ ] Check against project conventions

Phase 3 Complete When:

• [ ] All issues are documented with locations
• [ ] Issues are categorized by type and severity
• [ ] You understand the root cause of each issue

Phase 4: Plan Refactoring

Objective: Create a safe, incremental execution plan.

1. Create todo list for multi-step refactorings (use TodoWrite tool)

   • Break large refactorings into small, verifiable steps
   • Each step should be independently testable

2. Prioritize changes by impact and risk

   • High impact, low risk first (e.g., rename variables)
   • Defer high-risk changes (e.g., algorithm rewrites)

3. Consider backward compatibility

   • If this is a public API, can we deprecate gradually?
   • Do we need to support both old and new interfaces temporarily?

4. Plan verification steps

   • What will you check after each change?
   • Which tests must pass?

Example refactoring breakdown:

**Large task:** Extract user authentication logic into reusable hook

**Breakdown:**
1. Create new hook file with basic structure
2. Move authentication state management to hook
3. Move login/logout handlers to hook
4. Extract token refresh logic
5. Replace inline logic in components with hook calls
6. Remove old duplicated code
7. Add tests for hook

Phase 4 Complete When:

• [ ] Refactoring is broken into small steps
• [ ] Each step has clear verification criteria
• [ ] Dependencies between steps are identified
• [ ] Risk mitigation strategies are planned

Phase 5: Execute & Verify

Objective: Make changes safely with continuous verification.

Execution principles:

1. Make one change at a time

   • Single responsibility per edit
   • Commit frequently if using git

2. Run tests after each change

   • Don't accumulate untested changes
   • Catch regressions immediately

3. Check TypeScript compilation

   • tsc --noEmit or equivalent
   • Fix type errors before proceeding

4. Verify behavior unchanged

   • Run the application manually if needed
   • Confirm user-facing behavior is identical

Verification checklist per step:

• [ ] Tests pass (npm test or equivalent)
• [ ] TypeScript compiles without errors
• [ ] No new linting warnings
• [ ] Behavior manually verified (if high-risk change)

If something breaks:

• STOP immediately
• Use systematic-debugging skill to investigate
• Don't proceed until root cause is understood
• Consider reverting if fix is unclear

Phase 5 Complete When:

• [ ] All planned changes are implemented
• [ ] All tests pass
• [ ] No new type errors or warnings
• [ ] Code follows project conventions
• [ ] Behavior is verified unchanged

Red Flags - STOP and Reassess

If you catch yourself:

• Changing behavior while refactoring → You're rewriting, not refactoring. Separate behavior changes from refactoring.
• "I'll just skip the tests this time" → Tests are your safety net. No tests = no safe refactoring.
• Adding complexity to remove complexity → Over-engineering. Simpler solution probably exists.
• "This is clever!" → Code should be clear, not clever. Clever code is hard to maintain.
• Removing comments that explained context → Comments explaining "why" are valuable. Keep them.
• Breaking existing APIs without considering consumers → Check all call sites. Plan deprecation if needed.
• Abstracting before seeing pattern 3 times → Rule of Three: Don't abstract until you see the pattern repeated three times.

ALL of these mean: STOP. Reassess your approach.

React/TypeScript Patterns

For TypeScript/React projects, see references/react-typescript.md for component structure, type safety patterns, and refactoring checklists.

Common Refactoring Patterns - Quick Reference

| Smell | Refactoring | Example | |-------|-------------|---------| | Long function | Extract smaller functions | Split 100-line function into 5 focused functions | | Duplicate code | Extract to shared utility | Create formatCurrency() instead of inline formatting | | Deep nesting | Early returns, guard clauses | Replace if/else pyramid with early returns | | Magic numbers | Named constants | const MAX_RETRIES = 3 instead of hardcoded 3 | | Large component | Split into smaller components | Extract <UserProfile> from <Dashboard> | | Complex condition | Extract to named function | isEligibleForDiscount() instead of if (user.age > 65 && user.isPremium && ...) | | Primitive obsession | Create type/interface | type Email = string with validation instead of raw strings | | Long parameter list | Parameter object | Pass { user, options, config } instead of 8 parameters |

Integration with Other Skills

This skill requires using:

• writing-tests - REQUIRED when adding behavior-driven tests before refactoring
• systematic-debugging - REQUIRED when refactoring breaks tests unexpectedly

Complementary skills:

• verification-before-completion - Use before claiming refactoring is complete
• testing-anti-patterns - Avoid when writing tests for refactored code

When to Stop Refactoring

You've refactored enough when:

• ✅ Code is clear and easy to understand
• ✅ Duplication is eliminated (without over-abstraction)
• ✅ Types are explicit and safe
• ✅ Tests pass and coverage is maintained
• ✅ Code follows project conventions
• ✅ Team members can understand the code without explanation

Don't refactor further if:

• You're making code "elegant" but harder to understand
• You're abstracting before seeing the pattern 3 times
• You're refactoring code that works well and won't change soon
• You're optimizing performance without measuring first

Communication

When proposing refactorings:

1. Explain what problems you're solving

   • "This function is 150 lines and does 5 different things"
   • Not just "This code could be better"

2. Show before/after comparisons

   • Demonstrate the improvement concretely
   • Highlight reduced complexity metrics

3. Highlight risks and breaking changes

   • "This changes the public API"
   • "Consumers will need to update imports"

4. Suggest incremental steps for large refactorings

   • "We can do this in 3 phases over 2 weeks"
   • Make large changes less scary

5. Challenge your own assumptions

   • "Is this really better, or just different?"
   • "Am I solving a real problem?"

Key Principles Summary

1. Preserve behavior - Tests must verify same outcomes before and after
2. Test first - Behavior-driven tests are your safety net
3. Small steps - One change at a time, verify continuously
4. Clarity over cleverness - Code should be obvious, not impressive
5. Rule of Three - Don't abstract until you see the pattern three times
6. Safety over elegance - Working code is better than beautiful code

Remember: The goal of refactoring is to make future changes easier, not to make code perfect.