refactor

What this skill does

# Refactor Skill

When refactoring code, follow this structured process. The golden rule: change structure without changing behavior. Every refactoring should be verifiable by existing tests — if there are no tests, write them first.

## 1. Pre-Refactor Analysis

Before changing anything, understand the current state:

### Read and Map the Code
```bash
# Understand the file and its dependencies
cat [target-file]

# Find what imports this file (who depends on it)
grep -rn "import.*from.*[filename]" --include="*.ts" --include="*.js" --include="*.py" src/ 2>/dev/null
grep -rn "require.*[filename]" --include="*.js" src/ 2>/dev/null

# Find what this file imports (what it depends on)
grep -E "^import|^from|require\(" [target-file]

# Check for existing tests
find . -name "*[filename]*test*" -o -name "*[filename]*spec*" -o -name "test_*[filename]*" 2>/dev/null
```

### Measure Current Complexity
```bash
# Line count per function (rough estimate)
grep -n "function\|const.*=.*=>\|def \|func \|fn \|pub fn\|class " [target-file]

# File line count
wc -l [target-file]

# Nesting depth (count indentation levels)
awk '{ match($0, /^[[:space:]]*/); print RLENGTH/2, $0 }' [target-file] | sort -rn | head -10
```

### Verify Test Coverage
```bash
# Run existing tests to establish a baseline
npm test -- --coverage --testPathPattern=[filename] 2>/dev/null
python -m pytest --cov=[module] tests/test_[filename].py 2>/dev/null
go test -cover ./[package]/ 2>/dev/null

# If no tests exist, flag this immediately
```

**CRITICAL**: If no tests cover the code being refactored, the FIRST step is to add characterization tests that capture the current behavior. Never refactor untested code.

## 2. Code Smell Detection

Scan for these common smells, grouped by severity:

### 🔴 Critical Smells — Refactor First

#### Long Functions / Methods
```
Symptom: Function > 30 lines or does more than one thing
Detection: Count lines, look for multiple levels of abstraction
```
```bash
# Find long functions (rough heuristic)
awk '/^[[:space:]]*(export )?(async )?(function|const.*=>|def |func |fn |pub fn)/{name=$0; count=0} {count++} /^[[:space:]]*\}/{if(count>30) print count, name}' [target-file]
```

#### God Class / God Module
```
Symptom: File > 500 lines, class with 10+ methods, module that does everything
Detection: Line count, method count, number of imports
```
```bash
# Count methods in classes
grep -c "^\s*\(public\|private\|protected\|async\)\?\s*\w\+\s*(" [target-file]

# Count imports (high import count = too many responsibilities)
grep -c "^import\|^from.*import\|require(" [target-file]
```

#### Deeply Nested Code
```
Symptom: 4+ levels of indentation, nested if/else/for/try
Detection: Indentation analysis

// 🔴 SMELL — deeply nested
function processOrder(order) {
  if (order) {
    if (order.items.length > 0) {
      for (const item of order.items) {
        if (item.quantity > 0) {
          if (item.inStock) {
            // actual logic buried 5 levels deep
          }
        }
      }
    }
  }
}

// ✅ REFACTORED — early returns + extracted functions
function processOrder(order) {
  if (!order) return;
  if (order.items.length === 0) return;

  const validItems = order.items.filter(item => item.quantity > 0 && item.inStock);
  validItems.forEach(processItem);
}
```

#### Duplicated Code
```
Symptom: Same logic copy-pasted in multiple places
Detection: Similar code blocks, only differing in small details
```
```bash
# Find duplicate lines (rough detection)
sort [target-file] | uniq -d | grep -v "^$\|^import\|^//\|^#\|^\s*$" | head -20

# Find similar functions across files
grep -rn "function.*validate\|def.*validate\|func.*Validate" --include="*.ts" --include="*.py" --include="*.go" src/ 2>/dev/null
```

#### Primitive Obsession
```
Symptom: Using raw strings/numbers instead of domain types

// 🔴 SMELL — raw strings everywhere
function createUser(name: string, email: string, role: string, status: string) { ... }
function sendEmail(to: string, subject: string, body: string, priority: string) { ... }

// ✅ REFACTORED — domain types
function createUser(input: CreateUserInput): User { ... }
function sendEmail(message: EmailMessage): void { ... }

interface CreateUserInput {
  name: string;
  email: Email;        // validated email type
  role: UserRole;      // 'admin' | 'editor' | 'viewer'
  status: UserStatus;  // 'active' | 'inactive' | 'suspended'
}
```

### 🟡 Warning Smells — Refactor Soon

#### Feature Envy
```
Symptom: A function that uses more data from another class/module than its own

// 🔴 SMELL — OrderPrinter knows too much about Order internals
class OrderPrinter {
  print(order: Order) {
    const subtotal = order.items.reduce((sum, i) => sum + i.price * i.quantity, 0);
    const tax = subtotal * order.taxRate;
    const shipping = order.weight > 10 ? 15.99 : 5.99;
    const total = subtotal + tax + shipping;
    // ...
  }
}

// ✅ REFACTORED — move calculation to Order
class Order {
  get subtotal() { return this.items.reduce((sum, i) => sum + i.price * i.quantity, 0); }
  get tax() { return this.subtotal * this.taxRate; }
  get shippingCost() { return this.weight > 10 ? 15.99 : 5.99; }
  get total() { return this.subtotal + this.tax + this.shippingCost; }
}

class OrderPrinter {
  print(order: Order) {
    // Just use order.total, order.subtotal, etc.
  }
}
```

#### Long Parameter Lists
```
Symptom: Function with 4+ parameters

// 🔴 SMELL
function createUser(name, email, role, department, manager, startDate, salary, location) { ... }

// ✅ REFACTORED — parameter object
function createUser(input: CreateUserInput) { ... }
```

#### Switch/If-Else Chains
```
Symptom: Long switch or if/else chains that map types to behavior

// 🔴 SMELL
function calculateDiscount(customerType: string, amount: number): number {
  if (customerType === 'gold') return amount * 0.2;
  if (customerType === 'silver') return amount * 0.1;
  if (customerType === 'bronze') return amount * 0.05;
  if (customerType === 'employee') return amount * 0.3;
  return 0;
}

// ✅ REFACTORED — strategy map
const DISCOUNT_RATES: Record<string, number> = {
  gold: 0.2,
  silver: 0.1,
  bronze: 0.05,
  employee: 0.3,
};

function calculateDiscount(customerType: string, amount: number): number {
  return amount * (DISCOUNT_RATES[customerType] ?? 0);
}
```

#### Boolean Blindness
```
Symptom: Functions that take boolean flags to change behavior

// 🔴 SMELL — what does `true` mean here?
processOrder(order, true, false, true);

// ✅ REFACTORED — named options
processOrder(order, {
  expedited: true,
  giftWrap: false,
  sendNotification: true,
});
```

#### Magic Numbers and Strings
```
Symptom: Unexplained literal values in code

// 🔴 SMELL
if (retryCount > 3) { ... }
if (user.role === 'admin') { ... }
const timeout = 30000;

// ✅ REFACTORED
const MAX_RETRIES = 3;
const ROLES = { ADMIN: 'admin', EDITOR: 'editor' } as const;
const REQUEST_TIMEOUT_MS = 30_000;
```

#### Dead Code
```
Symptom: Unreachable code, unused variables, commented-out blocks
```
```bash
# Find unused exports (TypeScript)
npx ts-prune 2>/dev/null

# Find unused variables
npx eslint --rule '{"no-unused-vars": "error"}' [target-file] 2>/dev/null

# Find commented-out code blocks
grep -n "^\s*//.*function\|^\s*//.*const\|^\s*//.*class\|^\s*#.*def " [target-file]
```

### 🟢 Minor Smells — Refactor When Convenient

#### Inconsistent Naming
```
Symptom: Mixed conventions in the same codebase

// 🔴 SMELL — mixed naming
const user_name = getUserName();
const userEmail = get_user_email();
const UserRole = fetchRole();

// ✅ REFACTORED — consistent camelCase
const userName = getUserName();
const userEmail = getUserEmail();
const userRole = fetchRole();
```

#### Comments That Should Be Code
```
Symptom: Comments explaining WHAT, not WHY

// 🔴 SMELL — comment restates the code
// Check if user is admin
if (user.role === 'admin') { ... }

// ✅ REFACTORED — self-documenting code
if (user.isAdmin()) { ... }

// 🔴 SMELL — comment as sectio