Refactoring Skill

Guide for identifying and consolidating duplicate code while maintaining codebase quality.

Core Principle: Discuss Before Deciding

IMPORTANT: Always discuss refactoring decisions with the user before implementing. Use AskUserQuestion to:

• •Confirm whether code is truly duplicate vs. intentionally similar
• •Validate proposed extraction patterns
• •Get approval on naming and module placement
• •Check if there are historical reasons for current structure

Critical Anti-Pattern: Dead Helpers

The most dangerous duplication: A tested helper that production doesn't use.

❌ What we found:
   marchingSquares.js: renderMultiContour() - tested, used by Storybook
   main.jsx: inline loop doing same thing - untested, used in production

   Result: Tests pass, but production runs different code.

How to Detect

Before any refactoring work, check for unused helpers:

grep -l "export function" src/render/*.js | while read f; do
  funcs=$(grep -o "export function [a-zA-Z]*" "$f" | cut -d' ' -f3)
  for func in $funcs; do
    if ! grep -rq "$func" src/main.jsx src/stories/; then
      echo "⚠️  $f: $func exported but not used in production"
    fi
  done
done

How to Fix

1. •If helper is correct: Update main.jsx to use it, delete inline code
2. •If inline is correct: Delete the unused helper and its tests
3. •Never leave both: One implementation, used everywhere

Duplicate Code Patterns to Look For

1. Copy-Paste Functions

Similar functions with minor variations:

// Suspicious pattern - similar structure, different constants
function calculateFoamA(x, y) {
  return x * 0.5 + y * FOAM_FACTOR_A;
}
function calculateFoamB(x, y) {
  return x * 0.5 + y * FOAM_FACTOR_B;
}

Before refactoring, ASK:

• •Are these intentionally separate for performance/clarity?
• •Should they share logic or remain independent?

2. Repeated Logic Blocks

Same operations appearing in multiple places:

// Pattern: repeated clamping/normalization
const value = Math.max(0, Math.min(1, rawValue));  // appears in 5 files

3. Similar Data Transformations

Multiple functions doing equivalent transformations on different data shapes.

4. Parallel Structures

Similar class/module structures that could share a base:

// src/render/foamRenderer.js
// src/render/waveRenderer.js
// Both have: init(), update(), render(), cleanup()

Refactoring Decision Framework

When TO Consolidate (Rule of 3+)

• •Same logic appears 3+ times
• •Changes to one copy usually require changes to others
• •The abstraction has a clear, meaningful name
• •User agrees the duplication is problematic

When NOT to Consolidate

• •Code is similar but serves different purposes
• •Premature abstraction would obscure intent
• •Performance-critical paths benefit from inlining
• •User prefers explicit over DRY in this case

Discovery Process

1. •Identify candidates: Use grep/glob to find similar patterns
2. •Assess scope: How many instances? How similar?
3. •Discuss with user: Present findings, propose options
4. •Get explicit approval: Before any extraction
5. •Plan the refactor: Create plan document if significant
6. •Implement incrementally: One extraction at a time
7. •Verify: Run tests after each change

Codebase-Specific Patterns

This project uses:

• •Vanilla JavaScript (no TypeScript)
• •Canvas 2D rendering
• •Physics simulation with tight loops
• •Vitest for testing

Performance considerations:

• •Rendering code may intentionally inline for speed
• •Physics calculations may duplicate for cache locality
• •Always check if duplication is intentional optimization

Discussion Templates

When presenting refactoring opportunities:

I found [N] similar code blocks:
- [file1:line] - [brief description]
- [file2:line] - [brief description]

Options:
A) Extract to shared function in [module]
B) Keep separate (they serve different purposes)
C) Other approach

Which direction would you prefer?

Integration with Plans

For significant refactoring:

1. •Create plan in plans/refactoring/ using /feat refactoring/[name]
2. •Document current state, proposed changes, affected files
3. •Track progress via plan status

Commands Reference

# Search for similar functions
grep -r "function.*calculate" src/

# Find files with similar structure
find src/ -name "*.js" -exec grep -l "pattern" {} \;

# Check test coverage before refactoring
npm test

# Verify after changes
npm run lint && npm test

Checklist Before Refactoring

• • Discussed approach with user
• • Confirmed duplication is problematic (not intentional)
• • Tests exist for affected code
• • User approved proposed pattern/naming
• • Plan document created (if significant change)

CRITICAL: Testing After Refactoring

After EVERY refactoring change, run tests in this order:

# 1. Lint first (fast feedback)
npm run lint

# 2. Smoke test - verify app actually loads
npx playwright test tests/smoke.spec.js:3

# 3. Specific tests for changed files
npx vitest run src/path/changed-file.test.js

# 4. Full suite only if needed
npm test

DO NOT skip the smoke test - unit tests can pass while the app is broken (e.g., broken imports not exercised in unit tests).

DO NOT run the full test suite first - start with specific tests for faster iteration.