refactor

What this skill does

# Refactoring Skill

Improve code structure and quality while preserving behavior.

## Name

han-core:refactor - Restructure code to improve quality without changing behavior

## Synopsis

```
/refactor [arguments]
```

## Core Principle

**Tests are your safety net.** Never refactor without tests.

## The Refactoring Cycle

1. **Ensure tests exist and pass**
2. **Make ONE small change**
3. **Run tests** (must still pass)
4. **Commit** (keep changes isolated)
5. **Repeat**

**Each step must be reversible.** If tests fail, revert and try smaller change.

## Pre-Refactoring Checklist

**STOP if any of these are false:**

- [ ] Tests exist for code being refactored
- [ ] All tests currently pass
- [ ] Understand what code does
- [ ] External behavior will remain unchanged
- [ ] Have time to do this properly (not rushing)

**If no tests exist:**

1. Add tests first
2. Verify tests pass
3. THEN refactor

## When to Refactor

### Code Smells That Suggest Refactoring

**Readability issues:**

- Long functions (> 50 lines)
- Deep nesting (> 3 levels)
- Unclear naming
- Magic numbers
- Complex conditionals

**Maintainability issues:**

- Duplication (same code in multiple places)
- God classes (too many responsibilities)
- Feature envy (method uses another class more than its own)
- Data clumps (same groups of parameters passed around)

**Complexity issues:**

- Cyclomatic complexity > 10
- Too many dependencies
- Tightly coupled code
- Difficult to test

### When NOT to Refactor

- **No tests exist** (add tests first)
- **Under deadline pressure** (defer to later)
- **Code works and is readable** (don't over-engineer)
- **Changing external behavior** (that's not refactoring, that's a feature/fix)
- **Right before release** (too risky)

## Classic Refactorings

### Extract Function

**Problem:** Function does too many things

```typescript
// Before: Long function doing multiple things
function processOrder(order: Order) {
  // Validate order
  if (!order.items || order.items.length === 0) {
    throw new Error('Empty order')
  }
  if (!order.customer || !order.customer.email) {
    throw new Error('Invalid customer')
  }

  // Calculate totals
  let subtotal = 0
  for (const item of order.items) {
    subtotal += item.price * item.quantity
  }
  const tax = subtotal * 0.08
  const shipping = subtotal > 50 ? 0 : 9.99
  const total = subtotal + tax + shipping

  // Save to database
  return database.save({
    ...order,
    subtotal,
    tax,
    shipping,
    total
  })
}

// After: Extracted into focused functions
function processOrder(order: Order) {
  validateOrder(order)
  const totals = calculateTotals(order)
  return saveOrder(order, totals)
}

function validateOrder(order: Order): void {
  if (!order.items || order.items.length === 0) {
    throw new Error('Empty order')
  }
  if (!order.customer || !order.customer.email) {
    throw new Error('Invalid customer')
  }
}

function calculateTotals(order: Order) {
  const subtotal = order.items.reduce(
    (sum, item) => sum + item.price * item.quantity,
    0
  )
  const tax = subtotal * 0.08
  const shipping = subtotal > 50 ? 0 : 9.99
  const total = subtotal + tax + shipping

  return { subtotal, tax, shipping, total }
}

function saveOrder(order: Order, totals: Totals) {
  return database.save({ ...order, ...totals })
}
```

**Benefits:** Each function has single responsibility, easier to test, easier to understand

### Extract Variable

**Problem:** Complex expression that's hard to understand

```typescript
// Before: Dense, hard to parse
if (user.age >= 18 && user.country === 'US' && !user.banned && user.verified) {
  // ...
}

// After: Intent is clear
const isAdult = user.age >= 18
const isUSResident = user.country === 'US'
const hasGoodStanding = !user.banned && user.verified
const canPurchase = isAdult && isUSResident && hasGoodStanding

if (canPurchase) {
  // ...
}
```

**Benefits:** Self-documenting, easier to debug, easier to modify

### Inline Function/Variable

**Problem:** Unnecessary indirection that doesn't add clarity

```typescript
// Before: Over-abstraction
function getTotal(order: Order) {
  return calculateTotalAmount(order)
}

function calculateTotalAmount(order: Order) {
  return order.subtotal + order.tax
}

// After: Inline the unnecessary layer
function getTotal(order: Order) {
  return order.subtotal + order.tax
}
```

**When to inline:** Abstraction doesn't add value, makes code harder to follow

### Rename

**Problem:** Unclear or misleading names

```typescript
// Before: Unclear
function proc(d: any) {
  const r = d.x * d.y
  return r
}

// After: Self-explanatory
function calculateArea(dimensions: Dimensions) {
  const area = dimensions.width * dimensions.height
  return area
}
```

**Benefits:** Code is self-documenting, no need to guess what variables mean

### Replace Magic Number with Named Constant

**Problem:** Unexplained numbers in code

```typescript
// Before: What's 0.08? What's 9.99?
const tax = subtotal * 0.08
const shipping = subtotal > 50 ? 0 : 9.99

// After: Clear meaning
const TAX_RATE = 0.08
const FREE_SHIPPING_THRESHOLD = 50
const STANDARD_SHIPPING_COST = 9.99

const tax = subtotal * TAX_RATE
const shipping = subtotal > FREE_SHIPPING_THRESHOLD ? 0 : STANDARD_SHIPPING_COST
```

### Remove Duplication

**Problem:** Same code in multiple places

```typescript
// Before: Duplication
function formatUserName(user: User) {
  return `${user.firstName} ${user.lastName}`.trim()
}

function formatAdminName(admin: Admin) {
  return `${admin.firstName} ${admin.lastName}`.trim()
}

function formatAuthorName(author: Author) {
  return `${author.firstName} ${author.lastName}`.trim()
}

// After: One implementation
function formatFullName(person: { firstName: string; lastName: string }) {
  return `${person.firstName} ${person.lastName}`.trim()
}

// Usage
formatFullName(user)
formatFullName(admin)
formatFullName(author)
```

### Simplify Conditional

**Problem:** Complex nested if/else

```typescript
// Before: Nested conditionals
function getShippingCost(order: Order) {
  if (order.total > 100) {
    return 0
  } else {
    if (order.items.length > 5) {
      return 5.99
    } else {
      if (order.weight > 10) {
        return 15.99
      } else {
        return 9.99
      }
    }
  }
}

// After: Early returns, flat structure
function getShippingCost(order: Order) {
  if (order.total > 100) return 0
  if (order.items.length > 5) return 5.99
  if (order.weight > 10) return 15.99
  return 9.99
}

// Or: Look-up table
const SHIPPING_RULES = [
  { condition: (o: Order) => o.total > 100, cost: 0 },
  { condition: (o: Order) => o.items.length > 5, cost: 5.99 },
  { condition: (o: Order) => o.weight > 10, cost: 15.99 },
]

function getShippingCost(order: Order) {
  const rule = SHIPPING_RULES.find(r => r.condition(order))
  return rule?.cost ?? 9.99
}
```

### Replace Conditional with Polymorphism

**Problem:** Type checks scattered throughout code

```typescript
// Before: Type checking everywhere
function calculatePrice(item: Item) {
  if (item.type === 'book') {
    return item.basePrice * 0.9  // 10% discount
  } else if (item.type === 'electronics') {
    return item.basePrice * 1.15  // 15% markup
  } else if (item.type === 'clothing') {
    return item.basePrice
  }
}

// After: Polymorphism
interface Item {
  calculatePrice(): number
}

class Book implements Item {
  calculatePrice() {
    return this.basePrice * 0.9
  }
}

class Electronics implements Item {
  calculatePrice() {
    return this.basePrice * 1.15
  }
}

class Clothing implements Item {
  calculatePrice() {
    return this.basePrice
  }
}

// Usage: No type checking needed
const price = item.calculatePrice()
```

### Split Function

**Problem:** Function tries to do too many things

```typescript
// Before: Does validation, calculation, and saving
function processPayment(payment: Payment) {
  // Validation
  if (!payment.amount || payment.amount <= 0) {
    throw new Error('Invalid amount')