# Code Smells Reference

Comprehensive catalog of code smells with detection heuristics, refactoring prescriptions, tooling by language, and complexity metrics.

---

## Smell Catalog

### Long Function / Method

**Heuristic:** > 20 lines, > 5 levels of indentation, or cyclomatic complexity > 10.

**Why it's a smell:** Long functions do too many things. They are hard to name, hard to test, hard to reuse, and hard to understand. Each additional responsibility multiplies the cognitive load.

**Detection:**

```
Function length check
│
├─ Count lines (excluding blanks and comments)
│  ├─ < 10 lines → Fine
│  ├─ 10-20 lines → Monitor
│  ├─ 20-50 lines → Likely needs extraction
│  └─ > 50 lines → Almost certainly too long
│
└─ Count indentation levels
   ├─ 1-2 levels → Normal
   ├─ 3 levels → Borderline
   └─ 4+ levels → Extract inner blocks
```

**Refactoring options:**
- Extract Function (most common)
- Decompose Conditional (if/else chains)
- Replace Loop with Pipeline (map/filter/reduce)
- Replace Method with Method Object (when extraction needs too many params)

**Example -- Before:**

```python
def process_application(app):
    # Validate
    if not app.name:
        raise ValueError("Name required")
    if not app.email or "@" not in app.email:
        raise ValueError("Valid email required")
    if app.age < 18:
        raise ValueError("Must be 18+")

    # Score
    score = 0
    if app.gpa > 3.5:
        score += 30
    elif app.gpa > 3.0:
        score += 20
    else:
        score += 10

    if app.experience_years > 5:
        score += 40
    elif app.experience_years > 2:
        score += 25
    else:
        score += 10

    if app.has_certification:
        score += 20

    # Decide
    if score >= 70:
        status = "accepted"
    elif score >= 50:
        status = "waitlisted"
    else:
        status = "rejected"

    # Persist and notify
    app.score = score
    app.status = status
    db.save(app)
    send_email(app.email, f"Your application was {status}")
    return app
```

**Example -- After:**

```python
def process_application(app):
    validate_application(app)
    score = calculate_score(app)
    status = determine_status(score)
    return finalize_application(app, score, status)
```

---

### God Object / God Class

**Heuristic:** > 10 public methods, > 500 lines, > 7 dependencies injected, or the class name contains "Manager", "Handler", "Processor", "Service" without a specific domain qualifier.

**Why it's a smell:** A class that knows too much or does too much becomes a bottleneck. Every change risks breaking unrelated functionality. It attracts more responsibilities because "it already handles X, so let's add Y."

**Detection:**

```
Signs of a God Object
│
├─ Class has 10+ public methods
├─ Constructor takes 5+ dependencies
├─ Multiple unrelated groups of methods
│  (some deal with users, others with billing, others with notifications)
├─ Methods don't use most of the class's fields
│  (low cohesion -- methods only touch a subset of state)
├─ Class is imported by > 20 other files
└─ Changes to the class are in every PR
```

**Refactoring options:**
- Extract Class (split by responsibility)
- Extract Interface (define role-specific interfaces)
- Move Method (move methods to the class whose data they use)
- Facade Pattern (keep the god object as a thin coordinator)

**TypeScript example -- identifying responsibilities:**

```typescript
// BEFORE: UserManager does everything
class UserManager {
  // Group 1: Authentication
  login(email, password) { /* ... */ }
  logout(userId) { /* ... */ }
  resetPassword(email) { /* ... */ }

  // Group 2: Profile management
  updateProfile(userId, data) { /* ... */ }
  uploadAvatar(userId, file) { /* ... */ }
  getPreferences(userId) { /* ... */ }

  // Group 3: Billing
  createSubscription(userId, plan) { /* ... */ }
  cancelSubscription(userId) { /* ... */ }
  processPayment(userId, amount) { /* ... */ }

  // Group 4: Notifications
  sendWelcomeEmail(userId) { /* ... */ }
  sendInvoice(userId) { /* ... */ }
  updateNotificationPrefs(userId, prefs) { /* ... */ }
}

// AFTER: Split by responsibility
class AuthService { login, logout, resetPassword }
class ProfileService { updateProfile, uploadAvatar, getPreferences }
class BillingService { createSubscription, cancelSubscription, processPayment }
class NotificationService { sendWelcomeEmail, sendInvoice, updateNotificationPrefs }
```

---

### Feature Envy

**Heuristic:** A method accesses another object's data more than its own. Count field accesses -- if > 50% reference another class, the method probably belongs there.

**Why it's a smell:** The method is in the wrong place. It has more affinity for another class's data, which means changes to that class's data structure will also require changing this method.

**Detection:**

```python
# Feature Envy: this method is in OrderService but only touches Product data
class OrderService:
    def calculate_discount(self, product):
        if product.category == "electronics" and product.price > 500:
            return product.price * product.bulk_discount_rate
        elif product.is_seasonal and product.days_until_expiry < 30:
            return product.price * 0.25
        return 0
```

**Fix: Move to the class whose data it uses:**

```python
class Product:
    def calculate_discount(self) -> float:
        if self.category == "electronics" and self.price > 500:
            return self.price * self.bulk_discount_rate
        elif self.is_seasonal and self.days_until_expiry < 30:
            return self.price * 0.25
        return 0
```

**Exception:** Feature envy is acceptable when:
- You intentionally keep logic separate from data (e.g., pure functions operating on DTOs)
- The "envied" class is a simple data transfer object with no behavior
- Moving the method would create a circular dependency

---

### Duplicate Code

**Heuristic:** Same logic in 2+ places, differing only in variable names or minor details. > 5 lines of near-identical code is a strong signal.

**Why it's a smell:** When you fix a bug in one copy, you must find and fix all copies. You will forget one. Duplicated code also inflates the codebase, making it harder to navigate.

**DRY vs WET vs AHA:**

```
Duplication Strategy
│
├─ 1 occurrence → Leave it alone
│
├─ 2 occurrences → Note it, but don't extract yet (WET: Write Everything Twice)
│  │  Reason: You don't yet know the right abstraction
│  └─ Exception: If the two are truly identical and likely to stay so, extract
│
├─ 3+ occurrences → Extract (AHA: Avoid Hasty Abstractions)
│  │  Now you have enough examples to see the real pattern
│  └─ Extract with parameters for the varying parts
│
└─ Wrong abstraction is worse than duplication
   If the shared code diverges, it's OK to un-DRY and duplicate again
```

**Detection tools:**

| Language | Tool | Command |
|----------|------|---------|
| JavaScript/TypeScript | jscpd | `npx jscpd src/ --min-lines 5` |
| Python | pylint | `pylint --disable=all --enable=duplicate-code src/` |
| Multi-language | PMD CPD | `pmd cpd --minimum-tokens 50 --dir src/` |
| Any | ast-grep | Write a pattern to find structural duplicates |
| IDE | IntelliJ | Analyze > Locate Duplicates |

**Example -- Extract with parameterization:**

```typescript
// BEFORE: Two near-identical functions
function sendWelcomeEmail(user: User) {
  const template = loadTemplate('welcome');
  const html = render(template, { name: user.name, date: new Date() });
  await mailer.send({ to: user.email, subject: 'Welcome!', html });
  await analytics.track('email_sent', { type: 'welcome', userId: user.id });
}

function sendPasswordResetEmail(user: User, token: string) {
  const template = loadTemplate('password-reset');
  const html = render(template, { name: user.name, token, date: new Date() });
  await mailer.send({ to: user.email, subject: 'Password Reset', html });
  await analytics.track('email_sent', { type: 'password-reset', userId: user.id });
}

// AFTER: Parameterized
interface EmailParams {
  templateName: string;
  subject: string;
  extraData?: Record<string, unknown>;
}

async function sendEmail(user: User, params: EmailParams) {
  const template = loadTemplate(params.templateName);
  const html = render(template, { name: user.name, date: new Date(), ...params.extraData });
  await mailer.send({ to: user.email, subject: params.subject, html });
  await analytics.track('email_sent', { type: params.templateName, userId: user.id });
}

// Usage
await sendEmail(user, { templateName: 'welcome', subject: 'Welcome!' });
await sendEmail(user, { templateName: 'password-reset', subject: 'Password Reset', extraData: { token } });
```

---

### Deep Nesting

**Heuristic:** > 3 levels of indentation from nesting if/for/while/try blocks.

**Why it's a smell:** Each level of nesting adds cognitive load. The reader must mentally track every condition that led to the current branch. Error handling and happy path become interleaved and hard to follow.

**Refactoring techniques:**

#### Guard Clauses (Early Returns)

```typescript
// BEFORE: nested
function processPayment(order: Order) {
  if (order) {
    if (order.items.length > 0) {
      if (order.paymentMethod) {
        if (order.total > 0) {
          // actual logic buried 4 levels deep
          return chargePayment(order);
        } else {
          throw new Error('Invalid total');
        }
      } else {
        throw new Error('No payment method');
      }
    } else {
      throw new Error('No items');
    }
  } else {
    throw new Error('No order');
  }
}

// AFTER: guard clauses
function processPayment(order: Order) {
  if (!order) throw new Error('No order');
  if (order.items.length === 0) throw new Error('No items');
  if (!order.paymentMethod) throw new Error('No payment method');
  if (order.total <= 0) throw new Error('Invalid total');

  return chargePayment(order);
}
```

#### Replace Nested Loops with Pipeline

```python
# BEFORE
results = []
for user in users:
    if user.is_active:
        for order in user.orders:
            if order.total > 100:
                results.append({
                    "user": user.name,
                    "order": order.id,
                    "total": order.total,
                })

# AFTER
results = [
    {"user": u.name, "order": o.id, "total": o.total}
    for u in users if u.is_active
    for o in u.orders if o.total > 100
]
```

#### Extract Inner Block

```go
// BEFORE
func processRecords(records []Record) error {
    for _, record := range records {
        if record.IsValid() {
            for _, field := range record.Fields {
                if field.NeedsTransform() {
                    // 20 lines of transformation logic
                }
            }
        }
    }
    return nil
}

// AFTER
func processRecords(records []Record) error {
    for _, record := range records {
        if !record.IsValid() {
            continue
        }
        if err := transformFields(record.Fields); err != nil {
            return err
        }
    }
    return nil
}

func transformFields(fields []Field) error {
    for _, field := range fields {
        if !field.NeedsTransform() {
            continue
        }
        if err := transformField(field); err != nil {
            return err
        }
    }
    return nil
}
```

---

### Primitive Obsession

**Heuristic:** Using raw strings, numbers, or booleans to represent domain concepts that have validation rules or behavior.

**Why it's a smell:** A string can hold any value, but an email address cannot. Primitive obsession means validation logic is scattered across every place the value is used, or worse, missing entirely.

**Examples:**

```typescript
// BEFORE: Primitives everywhere
function createUser(
  email: string,        // Could be "not-an-email"
  age: number,          // Could be -5 or 999
  role: string,         // Could be "superadmin-hacker"
  currency: string,     // Could be "DOGECOIN"
  amount: number,       // Dollars? Cents? Yen?
) { /* ... */ }

// AFTER: Value objects / branded types
type Email = string & { readonly __brand: 'Email' };
type Age = number & { readonly __brand: 'Age' };
type Role = 'admin' | 'editor' | 'viewer';
type Currency = 'USD' | 'EUR' | 'GBP';
interface Money { amount: number; currency: Currency; }

function createEmail(raw: string): Email {
  if (!/^[^@]+@[^@]+\.[^@]+$/.test(raw)) throw new Error('Invalid email');
  return raw as Email;
}

function createAge(raw: number): Age {
  if (raw < 0 || raw > 150) throw new Error('Invalid age');
  return raw as Age;
}
```

```rust
// Rust: newtypes
struct Email(String);
struct Age(u8);

impl Email {
    fn new(raw: &str) -> Result<Self, ValidationError> {
        if raw.contains('@') { Ok(Email(raw.to_string())) }
        else { Err(ValidationError::InvalidEmail) }
    }
}

impl Age {
    fn new(raw: u8) -> Result<Self, ValidationError> {
        if raw > 0 && raw < 150 { Ok(Age(raw)) }
        else { Err(ValidationError::InvalidAge) }
    }
}
```

```python
# Python: dataclass value objects
@dataclass(frozen=True)
class Email:
    value: str

    def __post_init__(self):
        if "@" not in self.value:
            raise ValueError(f"Invalid email: {self.value}")

@dataclass(frozen=True)
class Money:
    amount: Decimal
    currency: str

    def __add__(self, other: "Money") -> "Money":
        if self.currency != other.currency:
            raise ValueError("Cannot add different currencies")
        return Money(self.amount + other.amount, self.currency)
```

---

### Shotgun Surgery

**Heuristic:** A single logical change requires editing 5+ files. The opposite of god object -- responsibility is spread too thin.

**Why it's a smell:** Every change is a scavenger hunt. Easy to miss one of the N files that need updating, leading to inconsistencies.

**Detection:**

```
Signs of Shotgun Surgery
│
├─ Adding a new field requires changes in:
│  model + serializer + validator + API + UI + test + migration + docs
│
├─ git log shows that certain groups of files always change together
│  └─ git log --name-only --pretty=format: | sort | uniq -c | sort -rn
│
└─ Code review comments: "Did you update X too?"
```

**Refactoring options:**
- Move Method / Move Field to consolidate related logic
- Inline Class (merge overly-split classes)
- Create a module that owns the entire concept end-to-end

---

### Dead Code

**Heuristic:** Code that is never executed -- unused imports, unreachable branches, commented-out code, exports with no importers, functions never called.

**Why it's a smell:** Dead code confuses readers ("is this important?"), increases maintenance burden, and can mask bugs. It adds noise to search results and IDE navigation.

**Detection by language:**

```
Dead Code Detection
│
├─ TypeScript / JavaScript
│  ├─ knip → comprehensive (files, exports, deps, types)
│  │  └─ npx knip
│  ├─ ts-prune → unused exports
│  │  └─ npx ts-prune
│  ├─ eslint → unused vars, unreachable code
│  │  └─ no-unused-vars, no-unreachable
│  └─ webpack-bundle-analyzer → unused modules in bundle
│
├─ Python
│  ├─ vulture → unused functions, variables, imports
│  │  └─ vulture src/ --min-confidence 80
│  ├─ ruff → unused imports (F401), unreachable code
│  │  └─ ruff check --select F401,F811
│  └─ coverage.py → branches never executed in tests
│
├─ Go
│  ├─ Compiler → unused imports (error), unused vars (error)
│  ├─ staticcheck → unused functions, types, fields
│  │  └─ staticcheck ./...
│  └─ golangci-lint → deadcode, unused linters
│
├─ Rust
│  ├─ Compiler → dead_code, unused_imports warnings
│  │  └─ cargo build 2>&1 | rg 'warning.*unused'
│  └─ cargo-udeps → unused dependencies
│     └─ cargo +nightly udeps
│
└─ Manual checks
   ├─ Search for commented-out code blocks → delete them
   ├─ Search for TODO/FIXME referencing removed features
   └─ Check feature flags for permanently-off features
```

**Safe removal strategy:**

1. Verify the code is truly dead (not used via reflection, dynamic import, or external consumers)
2. Remove in small batches, run full test suite after each
3. One commit per logical group of dead code
4. Keep the PR focused: dead code removal only, no behavior changes

---

### Data Clumps

**Heuristic:** The same group of 3+ values appears together repeatedly as function parameters, constructor args, or data fields.

**Why it's a smell:** The group of values represents a concept that deserves its own name and type. Without it, you duplicate validation and the relationship between the values is implicit.

**Example:**

```typescript
// BEFORE: Data clump (lat, lng, altitude appear together repeatedly)
function calculateDistance(lat1: number, lng1: number, alt1: number,
                           lat2: number, lng2: number, alt2: number): number { /* ... */ }

function formatLocation(lat: number, lng: number, alt: number): string { /* ... */ }

function isWithinBounds(lat: number, lng: number, alt: number,
                         bounds: Bounds): boolean { /* ... */ }

// AFTER: Extract parameter object
interface GeoPoint {
  lat: number;
  lng: number;
  altitude: number;
}

function calculateDistance(from: GeoPoint, to: GeoPoint): number { /* ... */ }
function formatLocation(point: GeoPoint): string { /* ... */ }
function isWithinBounds(point: GeoPoint, bounds: Bounds): boolean { /* ... */ }
```

---

### Long Parameter List

**Heuristic:** A function takes more than 4 parameters. Even 3 can be too many if they are all the same type (easy to swap by mistake).

**Why it's a smell:** Hard to remember argument order. Easy to swap two arguments of the same type. Adding a parameter requires updating all call sites.

**Refactoring options:**

```
Too many parameters?
│
├─ Parameters represent a concept → Extract Parameter Object
│  (firstName, lastName, email, phone → ContactInfo)
│
├─ Parameters are configuration → Builder Pattern or Options Object
│  (timeout, retries, baseUrl, headers → ClientOptions)
│
├─ Some parameters are always the same → Set defaults or partial application
│  (logger, config are always the same → inject via constructor)
│
└─ Parameters are independent concerns → Split into multiple functions
   (validate(data, rules, locale, format) → validate(data, rules) + format(data, locale))
```

---

## Complexity Metrics

### Cyclomatic Complexity

Counts the number of independent paths through a function. Each `if`, `else`, `for`, `while`, `case`, `catch`, `&&`, `||` adds 1 to the count.

| Score | Risk | Action |
|-------|------|--------|
| 1-5 | Low | No action needed |
| 6-10 | Moderate | Consider simplification |
| 11-20 | High | Refactor: extract functions, decompose conditionals |
| > 20 | Very High | Mandatory refactoring |

**Measurement tools:**

| Language | Tool | Command |
|----------|------|---------|
| JavaScript | eslint complexity rule | `eslint --rule 'complexity: ["error", 10]'` |
| Python | radon | `radon cc src/ -a -nc` |
| Go | gocyclo | `gocyclo -over 10 .` |
| Rust | cargo-geiger | Measures unsafe code complexity |
| Multi | SonarQube | Dashboard with complexity metrics |

### Cognitive Complexity

A newer metric (from SonarSource) that better reflects human reading difficulty. Unlike cyclomatic complexity, it penalizes nesting and rewards linear flow.

Key differences from cyclomatic complexity:
- Nested `if` inside `for` scores higher than sequential `if` then `for`
- Early returns (`guard clauses`) reduce complexity
- `switch` counts once, not per case
- Boolean operator sequences (`a && b && c`) count once

### Coupling and Cohesion

```
Coupling (between modules) — LOWER is better
│
├─ Afferent Coupling (Ca): How many modules depend ON this module
│  High Ca = changing this module breaks many things
│
├─ Efferent Coupling (Ce): How many modules this module depends ON
│  High Ce = this module is fragile (many reasons to change)
│
└─ Instability = Ce / (Ca + Ce)
   0.0 = maximally stable (many dependents, few dependencies)
   1.0 = maximally unstable (few dependents, many dependencies)

Cohesion (within a module) — HIGHER is better
│
├─ Every method uses every field → perfectly cohesive
├─ Methods split into groups using different fields → low cohesion
│  → Split into multiple classes
└─ Measured by LCOM (Lack of Cohesion of Methods)
   LCOM = 0 → perfectly cohesive
   LCOM > 0 → methods don't relate to each other
```

---

## Detection Tools by Language

### JavaScript / TypeScript

| Tool | Detects | Install | Command |
|------|---------|---------|---------|
| **eslint** | Unused vars, complexity, unreachable code | `npm i -D eslint` | `eslint --rule 'complexity: ["error", 10]' src/` |
| **knip** | Unused files, exports, deps, types | `npm i -D knip` | `npx knip` |
| **ts-prune** | Unused exports | `npm i -D ts-prune` | `npx ts-prune` |
| **jscpd** | Copy-paste detection | `npm i -D jscpd` | `npx jscpd src/ --min-lines 5` |
| **SonarQube** | Comprehensive (complexity, duplication, smells) | Server install | Web dashboard |

### Python

| Tool | Detects | Install | Command |
|------|---------|---------|---------|
| **ruff** | Unused imports, vars, complexity, style | `pip install ruff` | `ruff check --select ALL src/` |
| **vulture** | Dead code (functions, vars, imports) | `pip install vulture` | `vulture src/ --min-confidence 80` |
| **radon** | Cyclomatic + Halstead complexity | `pip install radon` | `radon cc src/ -a -nc` |
| **pylint** | Duplicate code, design smells | `pip install pylint` | `pylint --disable=all --enable=R src/` |
| **wily** | Complexity trends over time | `pip install wily` | `wily build src/ && wily report src/module.py` |

### Go

| Tool | Detects | Install | Command |
|------|---------|---------|---------|
| **golangci-lint** | Meta-linter (50+ linters) | Binary install | `golangci-lint run` |
| **gocyclo** | Cyclomatic complexity | `go install` | `gocyclo -over 10 .` |
| **goconst** | Repeated strings/numbers | Part of golangci-lint | `golangci-lint run --enable goconst` |
| **dupl** | Duplicate code | Part of golangci-lint | `golangci-lint run --enable dupl` |
| **staticcheck** | Unused code, bugs, simplifications | `go install` | `staticcheck ./...` |

### Rust

| Tool | Detects | Install | Command |
|------|---------|---------|---------|
| **clippy** | Lint, style, complexity, correctness | Built-in | `cargo clippy -- -W clippy::all` |
| **cargo-udeps** | Unused dependencies | `cargo install` | `cargo +nightly udeps` |
| **cargo-geiger** | Unsafe code usage | `cargo install` | `cargo geiger` |
| **Compiler** | Dead code, unused imports/vars | Built-in | `#[deny(dead_code, unused)]` |

---

## Smell Prioritization

Not all smells are equally urgent. Prioritize based on impact:

```
Triage Smells
│
├─ Fix NOW (blocks work or causes bugs)
│  ├─ Dead code that confuses newcomers
│  ├─ Duplicate code that has already diverged (bug in one copy)
│  └─ God object that causes merge conflicts every sprint
│
├─ Fix SOON (increasing maintenance cost)
│  ├─ Long functions (> 50 lines)
│  ├─ Deep nesting (> 4 levels)
│  └─ Shotgun surgery (every feature touches 10 files)
│
├─ Fix LATER (annoyances, not blockers)
│  ├─ Primitive obsession (works but fragile)
│  ├─ Data clumps (inconvenient but functional)
│  └─ Moderate duplication (2 copies, stable)
│
└─ Maybe NEVER (acceptable trade-offs)
   ├─ Generated code (don't refactor auto-generated files)
   ├─ Legacy code with no tests (write tests first)
   └─ Code scheduled for replacement
```

---

## Anti-patterns in Smell Remediation

| Anti-pattern | Problem | Better Approach |
|--------------|---------|-----------------|
| Refactoring without tests | Cannot verify behavior is preserved | Write characterization tests first |
| Premature DRY | Wrong abstraction extracted from 2 examples | Wait for 3+ examples (AHA principle) |
| Big-bang refactor | Everything breaks at once, can't bisect | Small, incremental changes with tests after each |
| Gold plating | Refactoring beyond what is needed for the task | Refactor only the code you are actively working on |
| Refactoring old stable code "just because" | Risk with no business value | Only refactor when you need to change it |
| Introducing patterns without need | Design patterns are solutions to problems, not goals | Pattern should reduce complexity, not add it |

# Extract Patterns Reference

Detailed patterns for extracting code into well-named, well-scoped units. Each pattern includes when to apply, when NOT to apply, before/after examples in multiple languages, and common mistakes.

---

## Extract Function / Method

### When to Apply

- A block of code has a clear, nameable purpose
- The same logic appears in multiple places
- A function is too long and has identifiable sub-tasks
- A comment explains what the next block does (the comment becomes the function name)
- You want to test a piece of logic independently

### When NOT to Apply

- The code is already short and clear (1-3 lines with obvious intent)
- Extracting would require passing 5+ parameters (refactor coupling first)
- The code relies heavily on local mutable state that is hard to pass around
- The "extracted" function would only be called once and adds no clarity

### Parameter Design

```
How many inputs does the extracted code need?
│
├─ 0-3 values → Pass as individual parameters
│
├─ 4+ related values → Group into a parameter object / struct
│  └─ { user, permissions, settings } instead of (user, perms, theme, lang, tz)
│
├─ Values come from a shared context → Consider making it a method on that context
│
└─ Mix of config and data → Separate: config as constructor/init, data as method params
```

### Naming Guidelines

- Name describes WHAT, not HOW: `calculateShippingCost` not `loopThroughItemsAndSum`
- Use verbs for actions: `validate`, `transform`, `calculate`, `fetch`, `build`
- Use predicates for booleans: `isValid`, `hasPermission`, `canAccess`, `shouldRetry`
- Avoid generic names: `process`, `handle`, `do`, `run`, `execute` (too vague alone)
- Include the domain noun: `validateEmail` not just `validate`

### JavaScript / TypeScript

**Before:**

```typescript
async function processOrder(order: Order) {
  // Validate order items
  if (order.items.length === 0) {
    throw new Error('Order must have at least one item');
  }
  for (const item of order.items) {
    if (item.quantity <= 0) {
      throw new Error(`Invalid quantity for item ${item.id}`);
    }
    if (item.price < 0) {
      throw new Error(`Invalid price for item ${item.id}`);
    }
  }

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

  // Persist
  const savedOrder = await db.orders.create({
    ...order,
    subtotal,
    tax,
    shipping,
    total,
    status: 'confirmed',
  });

  // Notify
  await emailService.send(order.customerEmail, 'Order Confirmed', {
    orderId: savedOrder.id,
    total,
  });

  return savedOrder;
}
```

**After:**

```typescript
async function processOrder(order: Order): Promise<SavedOrder> {
  validateOrderItems(order.items);
  const totals = calculateOrderTotals(order.items);
  const savedOrder = await persistOrder(order, totals);
  await notifyCustomer(order.customerEmail, savedOrder.id, totals.total);
  return savedOrder;
}

function validateOrderItems(items: OrderItem[]): void {
  if (items.length === 0) {
    throw new Error('Order must have at least one item');
  }
  for (const item of items) {
    if (item.quantity <= 0) {
      throw new Error(`Invalid quantity for item ${item.id}`);
    }
    if (item.price < 0) {
      throw new Error(`Invalid price for item ${item.id}`);
    }
  }
}

interface OrderTotals {
  subtotal: number;
  tax: number;
  shipping: number;
  total: number;
}

function calculateOrderTotals(items: OrderItem[]): OrderTotals {
  const subtotal = items.reduce((sum, item) => sum + item.price * item.quantity, 0);
  const tax = subtotal * TAX_RATE;
  const shipping = subtotal > FREE_SHIPPING_THRESHOLD ? 0 : SHIPPING_COST;
  const total = subtotal + tax + shipping;
  return { subtotal, tax, shipping, total };
}

async function persistOrder(order: Order, totals: OrderTotals): Promise<SavedOrder> {
  return db.orders.create({ ...order, ...totals, status: 'confirmed' });
}

async function notifyCustomer(email: string, orderId: string, total: number): Promise<void> {
  await emailService.send(email, 'Order Confirmed', { orderId, total });
}
```

### Python

**Before:**

```python
def generate_report(users, start_date, end_date):
    # Filter active users in date range
    active_users = []
    for user in users:
        if user.is_active and start_date <= user.created_at <= end_date:
            if user.email_verified:
                active_users.append(user)

    # Calculate statistics
    total_revenue = 0
    for user in active_users:
        for order in user.orders:
            if order.status == "completed":
                total_revenue += order.total

    avg_revenue = total_revenue / len(active_users) if active_users else 0

    # Format output
    lines = [f"Report: {start_date} to {end_date}"]
    lines.append(f"Active Users: {len(active_users)}")
    lines.append(f"Total Revenue: ${total_revenue:.2f}")
    lines.append(f"Avg Revenue/User: ${avg_revenue:.2f}")
    return "\n".join(lines)
```

**After:**

```python
def generate_report(users: list[User], start_date: date, end_date: date) -> str:
    active_users = filter_active_users(users, start_date, end_date)
    revenue = calculate_total_revenue(active_users)
    avg_revenue = revenue / len(active_users) if active_users else 0
    return format_report(start_date, end_date, len(active_users), revenue, avg_revenue)


def filter_active_users(users: list[User], start: date, end: date) -> list[User]:
    return [
        u for u in users
        if u.is_active and u.email_verified and start <= u.created_at <= end
    ]


def calculate_total_revenue(users: list[User]) -> float:
    return sum(
        order.total
        for user in users
        for order in user.orders
        if order.status == "completed"
    )


def format_report(start: date, end: date, user_count: int, revenue: float, avg: float) -> str:
    return "\n".join([
        f"Report: {start} to {end}",
        f"Active Users: {user_count}",
        f"Total Revenue: ${revenue:.2f}",
        f"Avg Revenue/User: ${avg:.2f}",
    ])
```

### Go

**Before:**

```go
func HandleUpload(w http.ResponseWriter, r *http.Request) {
    file, header, err := r.FormFile("document")
    if err != nil {
        http.Error(w, "Failed to read file", http.StatusBadRequest)
        return
    }
    defer file.Close()

    if header.Size > 10*1024*1024 {
        http.Error(w, "File too large", http.StatusBadRequest)
        return
    }
    ext := filepath.Ext(header.Filename)
    if ext != ".pdf" && ext != ".docx" && ext != ".txt" {
        http.Error(w, "Unsupported file type", http.StatusBadRequest)
        return
    }

    data, err := io.ReadAll(file)
    if err != nil {
        http.Error(w, "Failed to read file", http.StatusInternalServerError)
        return
    }

    hash := sha256.Sum256(data)
    filename := fmt.Sprintf("%x%s", hash, ext)
    path := filepath.Join("uploads", filename)
    if err := os.WriteFile(path, data, 0644); err != nil {
        http.Error(w, "Failed to save file", http.StatusInternalServerError)
        return
    }

    json.NewEncoder(w).Encode(map[string]string{"path": path})
}
```

**After:**

```go
func HandleUpload(w http.ResponseWriter, r *http.Request) {
    file, header, err := r.FormFile("document")
    if err != nil {
        http.Error(w, "Failed to read file", http.StatusBadRequest)
        return
    }
    defer file.Close()

    if err := validateUpload(header); err != nil {
        http.Error(w, err.Error(), http.StatusBadRequest)
        return
    }

    path, err := saveFile(file, header.Filename)
    if err != nil {
        http.Error(w, "Failed to save file", http.StatusInternalServerError)
        return
    }

    json.NewEncoder(w).Encode(map[string]string{"path": path})
}

func validateUpload(header *multipart.FileHeader) error {
    if header.Size > maxUploadSize {
        return fmt.Errorf("file too large (max %d bytes)", maxUploadSize)
    }
    ext := filepath.Ext(header.Filename)
    if !allowedExtensions[ext] {
        return fmt.Errorf("unsupported file type: %s", ext)
    }
    return nil
}

func saveFile(file multipart.File, originalName string) (string, error) {
    data, err := io.ReadAll(file)
    if err != nil {
        return "", fmt.Errorf("reading file: %w", err)
    }
    hash := sha256.Sum256(data)
    ext := filepath.Ext(originalName)
    filename := fmt.Sprintf("%x%s", hash, ext)
    path := filepath.Join("uploads", filename)
    if err := os.WriteFile(path, data, 0644); err != nil {
        return "", fmt.Errorf("writing file: %w", err)
    }
    return path, nil
}
```

### Rust

**Before:**

```rust
fn process_csv(path: &str) -> Result<Vec<Record>, Box<dyn Error>> {
    let content = fs::read_to_string(path)?;
    let mut records = Vec::new();

    for (i, line) in content.lines().enumerate() {
        if i == 0 { continue; } // skip header
        let fields: Vec<&str> = line.split(',').collect();
        if fields.len() < 3 {
            eprintln!("Skipping malformed line {}: {}", i, line);
            continue;
        }
        let name = fields[0].trim().to_string();
        let age: u32 = match fields[1].trim().parse() {
            Ok(a) if a > 0 && a < 150 => a,
            _ => {
                eprintln!("Invalid age on line {}", i);
                continue;
            }
        };
        let email = fields[2].trim().to_string();
        if !email.contains('@') {
            eprintln!("Invalid email on line {}", i);
            continue;
        }
        records.push(Record { name, age, email });
    }
    Ok(records)
}
```

**After:**

```rust
fn process_csv(path: &str) -> Result<Vec<Record>, Box<dyn Error>> {
    let content = fs::read_to_string(path)?;
    let records = content
        .lines()
        .enumerate()
        .skip(1) // skip header
        .filter_map(|(i, line)| parse_record(i, line))
        .collect();
    Ok(records)
}

fn parse_record(line_num: usize, line: &str) -> Option<Record> {
    let fields: Vec<&str> = line.split(',').collect();
    if fields.len() < 3 {
        eprintln!("Skipping malformed line {}: {}", line_num, line);
        return None;
    }
    let name = fields[0].trim().to_string();
    let age = parse_age(fields[1].trim(), line_num)?;
    let email = parse_email(fields[2].trim(), line_num)?;
    Some(Record { name, age, email })
}

fn parse_age(s: &str, line_num: usize) -> Option<u32> {
    match s.parse::<u32>() {
        Ok(a) if a > 0 && a < 150 => Some(a),
        _ => {
            eprintln!("Invalid age on line {}", line_num);
            None
        }
    }
}

fn parse_email(s: &str, line_num: usize) -> Option<String> {
    if s.contains('@') {
        Some(s.to_string())
    } else {
        eprintln!("Invalid email on line {}", line_num);
        None
    }
}
```

### Common Mistakes

| Mistake | Problem | Fix |
|---------|---------|-----|
| Extracting with too many parameters | Function signature is unwieldy, hard to call | Group related params into a struct/object first |
| Naming the function after its implementation | `loopAndFilter` tells you nothing useful | Name after the WHAT: `filterActiveUsers` |
| Extracting one line into a function | Adds indirection without clarity | Only extract if the name adds understanding |
| Not returning a value | Using mutation/side effects when a return value is cleaner | Prefer pure functions that return results |
| Leaving the original code commented out | Clutters the file, confuses future readers | Delete it; git has history |

---

## Extract Component

### When to Apply

- A section of UI has its own state or lifecycle
- The same UI pattern appears in multiple places
- A component file exceeds 200 lines
- A piece of UI has a clear responsibility boundary
- You want to test UI logic independently

### When NOT to Apply

- The UI is a one-off, simple, and under 30 lines
- Extracting would require 10+ props (the component boundary is wrong)
- The "component" has no reuse potential and splitting hurts readability

### React

**Before:**

```tsx
function Dashboard({ user }: { user: User }) {
  const [searchTerm, setSearchTerm] = useState('');
  const [sortBy, setSortBy] = useState<'name' | 'date'>('date');

  const filteredProjects = useMemo(() => {
    return user.projects
      .filter(p => p.name.toLowerCase().includes(searchTerm.toLowerCase()))
      .sort((a, b) => sortBy === 'name'
        ? a.name.localeCompare(b.name)
        : b.createdAt.getTime() - a.createdAt.getTime()
      );
  }, [user.projects, searchTerm, sortBy]);

  return (
    <div>
      <h1>Welcome, {user.name}</h1>
      <div>
        <input
          value={searchTerm}
          onChange={e => setSearchTerm(e.target.value)}
          placeholder="Search projects..."
        />
        <select value={sortBy} onChange={e => setSortBy(e.target.value as 'name' | 'date')}>
          <option value="date">Sort by Date</option>
          <option value="name">Sort by Name</option>
        </select>
      </div>
      <ul>
        {filteredProjects.map(project => (
          <li key={project.id}>
            <h3>{project.name}</h3>
            <p>{project.description}</p>
            <span>{project.createdAt.toLocaleDateString()}</span>
            <span>{project.status}</span>
          </li>
        ))}
      </ul>
    </div>
  );
}
```

**After:**

```tsx
function Dashboard({ user }: { user: User }) {
  return (
    <div>
      <h1>Welcome, {user.name}</h1>
      <ProjectList projects={user.projects} />
    </div>
  );
}

// --- project-list.tsx ---

function ProjectList({ projects }: { projects: Project[] }) {
  const [searchTerm, setSearchTerm] = useState('');
  const [sortBy, setSortBy] = useState<SortField>('date');

  const filteredProjects = useFilteredProjects(projects, searchTerm, sortBy);

  return (
    <div>
      <ProjectFilters
        searchTerm={searchTerm}
        onSearchChange={setSearchTerm}
        sortBy={sortBy}
        onSortChange={setSortBy}
      />
      <ul>
        {filteredProjects.map(project => (
          <ProjectCard key={project.id} project={project} />
        ))}
      </ul>
    </div>
  );
}

// --- project-card.tsx ---

function ProjectCard({ project }: { project: Project }) {
  return (
    <li>
      <h3>{project.name}</h3>
      <p>{project.description}</p>
      <span>{project.createdAt.toLocaleDateString()}</span>
      <span>{project.status}</span>
    </li>
  );
}
```

### Vue

**Before:**

```vue
<template>
  <div>
    <input v-model="search" placeholder="Search..." />
    <ul>
      <li v-for="item in filteredItems" :key="item.id">
        <h3>{{ item.title }}</h3>
        <p>{{ item.body }}</p>
        <button @click="toggleFavorite(item.id)">
          {{ item.isFavorite ? 'Unfavorite' : 'Favorite' }}
        </button>
      </li>
    </ul>
  </div>
</template>
```

**After:**

```vue
<!-- SearchableList.vue -->
<template>
  <div>
    <SearchInput v-model="search" />
    <ItemCard
      v-for="item in filteredItems"
      :key="item.id"
      :item="item"
      @toggle-favorite="toggleFavorite"
    />
  </div>
</template>

<!-- ItemCard.vue -->
<template>
  <li>
    <h3>{{ item.title }}</h3>
    <p>{{ item.body }}</p>
    <button @click="$emit('toggle-favorite', item.id)">
      {{ item.isFavorite ? 'Unfavorite' : 'Favorite' }}
    </button>
  </li>
</template>
```

### Extraction Boundaries Decision

```
Should this be its own component?
│
├─ Does it have its own state? → YES, extract
├─ Is it reused in 2+ places? → YES, extract
├─ Is it > 50 lines of JSX/template? → Probably, extract
├─ Does it have a clear domain name? → YES, extract
├─ Would extracting require > 8 props? → NO, fix the boundary first
└─ Is it pure presentation, < 20 lines? → Probably not worth it
```

---

## Extract Hook / Composable

### When to Apply

- Stateful logic is duplicated across components
- A component has complex state management obscuring the template/JSX
- You want to test stateful logic without rendering
- The logic is reusable across different UI presentations

### When NOT to Apply

- The logic is simple and only used in one component
- The hook would just wrap a single useState/useRef with no additional logic
- The hook needs access to the component's render context

### React Custom Hook

**Before:**

```tsx
function UserProfile({ userId }: { userId: string }) {
  const [user, setUser] = useState<User | null>(null);
  const [loading, setLoading] = useState(true);
  const [error, setError] = useState<Error | null>(null);

  useEffect(() => {
    const controller = new AbortController();
    setLoading(true);
    setError(null);

    fetchUser(userId, { signal: controller.signal })
      .then(setUser)
      .catch(err => {
        if (!controller.signal.aborted) setError(err);
      })
      .finally(() => {
        if (!controller.signal.aborted) setLoading(false);
      });

    return () => controller.abort();
  }, [userId]);

  if (loading) return <Spinner />;
  if (error) return <ErrorMessage error={error} />;
  if (!user) return null;

  return <div>{user.name}</div>;
}
```

**After:**

```tsx
// hooks/use-user.ts
function useUser(userId: string) {
  const [user, setUser] = useState<User | null>(null);
  const [loading, setLoading] = useState(true);
  const [error, setError] = useState<Error | null>(null);

  useEffect(() => {
    const controller = new AbortController();
    setLoading(true);
    setError(null);

    fetchUser(userId, { signal: controller.signal })
      .then(setUser)
      .catch(err => {
        if (!controller.signal.aborted) setError(err);
      })
      .finally(() => {
        if (!controller.signal.aborted) setLoading(false);
      });

    return () => controller.abort();
  }, [userId]);

  return { user, loading, error };
}

// components/user-profile.tsx
function UserProfile({ userId }: { userId: string }) {
  const { user, loading, error } = useUser(userId);

  if (loading) return <Spinner />;
  if (error) return <ErrorMessage error={error} />;
  if (!user) return null;

  return <div>{user.name}</div>;
}
```

### Vue Composable

**Before:**

```vue
<script setup>
const items = ref([]);
const loading = ref(false);
const page = ref(1);
const hasMore = ref(true);

async function loadMore() {
  if (loading.value || !hasMore.value) return;
  loading.value = true;
  try {
    const result = await fetchItems({ page: page.value });
    items.value.push(...result.data);
    hasMore.value = result.hasMore;
    page.value++;
  } finally {
    loading.value = false;
  }
}

onMounted(loadMore);
</script>
```

**After:**

```typescript
// composables/use-paginated-list.ts
export function usePaginatedList<T>(fetcher: (page: number) => Promise<{ data: T[]; hasMore: boolean }>) {
  const items = ref<T[]>([]);
  const loading = ref(false);
  const page = ref(1);
  const hasMore = ref(true);

  async function loadMore() {
    if (loading.value || !hasMore.value) return;
    loading.value = true;
    try {
      const result = await fetcher(page.value);
      items.value.push(...result.data);
      hasMore.value = result.hasMore;
      page.value++;
    } finally {
      loading.value = false;
    }
  }

  onMounted(loadMore);

  return { items, loading, hasMore, loadMore };
}
```

```vue
<script setup>
const { items, loading, hasMore, loadMore } = usePaginatedList(
  (page) => fetchItems({ page })
);
</script>
```

---

## Extract Module

### When to Apply

- A file exceeds 300-500 lines
- A file contains multiple unrelated classes or groups of functions
- You want to lazy-load part of a file
- Testing requires importing the whole file when you only need a part

### When NOT to Apply

- The file is long but cohesive (one responsibility, everything interdependent)
- Splitting would create circular dependencies
- The file is generated code

### Strategy

```
Large File (500+ lines)
│
├─ Identify responsibility clusters
│  Group functions/classes by what they operate on
│
├─ Check dependency direction
│  Draw arrows: A uses B means A depends on B
│  If A and B depend on each other → shared types module first
│
├─ Create new files, one per cluster
│  Each file exports its public API
│
├─ Create barrel file (index.ts) if needed
│  Re-export public API for backward compatibility
│
└─ Update imports across codebase
   One file at a time, running tests after each
```

### Before (single large file):

```typescript
// utils.ts (600 lines)
export function formatDate(d: Date): string { /* ... */ }
export function parseDate(s: string): Date { /* ... */ }
export function daysBetween(a: Date, b: Date): number { /* ... */ }

export function formatCurrency(amount: number, currency: string): string { /* ... */ }
export function parseCurrency(s: string): number { /* ... */ }
export function convertCurrency(amount: number, from: string, to: string): number { /* ... */ }

export function validateEmail(email: string): boolean { /* ... */ }
export function validatePhone(phone: string): boolean { /* ... */ }
export function validateUrl(url: string): boolean { /* ... */ }
```

### After (split by responsibility):

```typescript
// utils/date.ts
export function formatDate(d: Date): string { /* ... */ }
export function parseDate(s: string): Date { /* ... */ }
export function daysBetween(a: Date, b: Date): number { /* ... */ }

// utils/currency.ts
export function formatCurrency(amount: number, currency: string): string { /* ... */ }
export function parseCurrency(s: string): number { /* ... */ }
export function convertCurrency(amount: number, from: string, to: string): number { /* ... */ }

// utils/validation.ts
export function validateEmail(email: string): boolean { /* ... */ }
export function validatePhone(phone: string): boolean { /* ... */ }
export function validateUrl(url: string): boolean { /* ... */ }

// utils/index.ts (barrel - backward compatible)
export * from './date';
export * from './currency';
export * from './validation';
```

### Circular Dependency Resolution

```
Problem: A imports from B, B imports from A

Solution 1: Extract shared types
├─ types.ts (shared interfaces/types)
├─ a.ts (imports from types.ts)
└─ b.ts (imports from types.ts)

Solution 2: Dependency inversion
├─ a.ts (defines interface, imports nothing from B)
├─ b.ts (implements A's interface)
└─ main.ts (wires A and B together)

Solution 3: Merge if truly coupled
└─ ab.ts (if A and B are one responsibility, keep them together)
```

---

## Extract Class / Service

### When to Apply

- A class has more than one reason to change (SRP violation)
- A group of functions all operate on the same data
- You need to swap implementations (strategy pattern, testing)
- Business logic is mixed with infrastructure (DB, HTTP, file I/O)

### When NOT to Apply

- The class is already cohesive and under 200 lines
- Extracting would create classes with only one method
- The "class" is really just a namespace for utility functions (use a module instead)

### TypeScript

**Before:**

```typescript
class OrderService {
  async createOrder(items: CartItem[], customer: Customer): Promise<Order> {
    // Validation
    if (items.length === 0) throw new Error('Cart is empty');
    for (const item of items) {
      const product = await this.db.products.findById(item.productId);
      if (!product) throw new Error(`Product ${item.productId} not found`);
      if (product.stock < item.quantity) throw new Error(`Insufficient stock`);
    }

    // Price calculation
    let subtotal = 0;
    for (const item of items) {
      const product = await this.db.products.findById(item.productId);
      subtotal += product!.price * item.quantity;
    }
    const discount = customer.tier === 'premium' ? subtotal * 0.1 : 0;
    const tax = (subtotal - discount) * 0.08;
    const total = subtotal - discount + tax;

    // Persistence
    const order = await this.db.orders.create({ items, customerId: customer.id, subtotal, discount, tax, total });

    // Notification
    await this.mailer.send(customer.email, 'Order Confirmed', { orderId: order.id, total });
    if (total > 500) {
      await this.slack.notify('#high-value-orders', `New order: $${total}`);
    }

    return order;
  }
}
```

**After:**

```typescript
class OrderService {
  constructor(
    private validator: OrderValidator,
    private calculator: PriceCalculator,
    private repository: OrderRepository,
    private notifier: OrderNotifier,
  ) {}

  async createOrder(items: CartItem[], customer: Customer): Promise<Order> {
    await this.validator.validateItems(items);
    const pricing = this.calculator.calculate(items, customer);
    const order = await this.repository.save(items, customer, pricing);
    await this.notifier.orderConfirmed(order, customer);
    return order;
  }
}

class OrderValidator {
  constructor(private productRepo: ProductRepository) {}

  async validateItems(items: CartItem[]): Promise<void> {
    if (items.length === 0) throw new Error('Cart is empty');
    for (const item of items) {
      const product = await this.productRepo.findById(item.productId);
      if (!product) throw new Error(`Product ${item.productId} not found`);
      if (product.stock < item.quantity) throw new Error('Insufficient stock');
    }
  }
}

class PriceCalculator {
  calculate(items: CartItem[], customer: Customer): OrderPricing {
    const subtotal = items.reduce((sum, i) => sum + i.price * i.quantity, 0);
    const discount = customer.tier === 'premium' ? subtotal * 0.1 : 0;
    const tax = (subtotal - discount) * TAX_RATE;
    return { subtotal, discount, tax, total: subtotal - discount + tax };
  }
}
```

### Python

**Before:**

```python
class ReportGenerator:
    def generate(self, data, format_type, output_path):
        # Data processing
        cleaned = [row for row in data if row.get("valid")]
        grouped = {}
        for row in cleaned:
            key = row["category"]
            grouped.setdefault(key, []).append(row)

        # Aggregation
        summary = {}
        for cat, rows in grouped.items():
            summary[cat] = {
                "count": len(rows),
                "total": sum(r["amount"] for r in rows),
                "average": sum(r["amount"] for r in rows) / len(rows),
            }

        # Formatting
        if format_type == "csv":
            output = self._to_csv(summary)
        elif format_type == "json":
            output = json.dumps(summary, indent=2)
        elif format_type == "html":
            output = self._to_html(summary)

        # File I/O
        with open(output_path, "w") as f:
            f.write(output)
```

**After:**

```python
class ReportGenerator:
    def __init__(self, processor: DataProcessor, formatter: ReportFormatter, writer: FileWriter):
        self.processor = processor
        self.formatter = formatter
        self.writer = writer

    def generate(self, data: list[dict], format_type: str, output_path: str) -> None:
        summary = self.processor.summarize(data)
        output = self.formatter.format(summary, format_type)
        self.writer.write(output, output_path)


class DataProcessor:
    def summarize(self, data: list[dict]) -> dict[str, CategorySummary]:
        cleaned = [row for row in data if row.get("valid")]
        grouped = self._group_by_category(cleaned)
        return {cat: self._aggregate(rows) for cat, rows in grouped.items()}

    def _group_by_category(self, rows):
        grouped = {}
        for row in rows:
            grouped.setdefault(row["category"], []).append(row)
        return grouped

    def _aggregate(self, rows):
        amounts = [r["amount"] for r in rows]
        return CategorySummary(count=len(rows), total=sum(amounts), average=sum(amounts) / len(rows))
```

---

## Extract Configuration

### When to Apply

- Magic numbers or strings scattered through code
- Environment-specific values hardcoded (URLs, ports, timeouts)
- Feature flags or A/B test conditions inline
- Same constants defined in multiple files

### When NOT to Apply

- The value is truly constant and universal (pi = 3.14159)
- The value is used exactly once and is self-documenting in context
- Extracting would require a complex configuration system for 2-3 values

### Before:

```typescript
async function fetchWithRetry(url: string) {
  for (let i = 0; i < 3; i++) {
    try {
      const response = await fetch(url, { timeout: 5000 });
      if (response.status === 429) {
        await sleep(1000 * Math.pow(2, i));
        continue;
      }
      return response;
    } catch {
      if (i === 2) throw new Error('Max retries exceeded');
      await sleep(1000 * Math.pow(2, i));
    }
  }
}
```

### After:

```typescript
// config/retry.ts
export const RETRY_CONFIG = {
  maxAttempts: 3,
  baseDelayMs: 1000,
  requestTimeoutMs: 5000,
  backoffMultiplier: 2,
  retryableStatusCodes: [429, 502, 503, 504],
} as const;

// lib/fetch-with-retry.ts
import { RETRY_CONFIG } from '../config/retry';

async function fetchWithRetry(url: string, config = RETRY_CONFIG) {
  for (let attempt = 0; attempt < config.maxAttempts; attempt++) {
    try {
      const response = await fetch(url, { timeout: config.requestTimeoutMs });
      if (config.retryableStatusCodes.includes(response.status)) {
        await sleep(config.baseDelayMs * Math.pow(config.backoffMultiplier, attempt));
        continue;
      }
      return response;
    } catch {
      if (attempt === config.maxAttempts - 1) throw new Error('Max retries exceeded');
      await sleep(config.baseDelayMs * Math.pow(config.backoffMultiplier, attempt));
    }
  }
}
```

### Configuration Extraction Checklist

```
[ ] Identified all magic numbers and strings
[ ] Grouped related config values into typed objects
[ ] Added sensible defaults (don't require config for common case)
[ ] Made config injectable for testing (parameter with default)
[ ] Documented units in names (timeoutMs, maxRetries, limitBytes)
[ ] Used const assertions or enums for type safety
[ ] Kept environment-specific values in env vars, not code
```

# Safe Refactoring Methodology Reference

Strategies for large and small refactorings that preserve behavior, minimize risk, and provide rollback safety.

---

## Test-Driven Refactoring

### The Core Loop

```
Red-Green-Refactor (for new code)
│
├─ RED: Write a failing test for the desired behavior
├─ GREEN: Write the simplest code that makes the test pass
└─ REFACTOR: Clean up while keeping tests green
   └─ This is where refactoring happens safely

Characterization-Then-Refactor (for existing code)
│
├─ Step 1: Write Characterization Tests
│  │  Run the existing code and capture its actual output
│  │  Assert on that output, even if it seems wrong
│  │  Goal: document what the code DOES, not what it SHOULD do
│  │
│  │  Example:
│  │  def test_calculate_tax_current_behavior():
│  │      # This may be "wrong" but it's what the code does today
│  │      assert calculate_tax(100) == 8.25  # captures actual behavior
│  │
│  └─ Coverage: ensure every branch you will touch is covered
│
├─ Step 2: Refactor Under Test Safety
│  │  Make one small change
│  │  Run all characterization tests
│  │  If tests pass → commit and continue
│  │  If tests fail → revert and try smaller change
│  └─ Never refactor and change behavior in the same step
│
├─ Step 3: Replace Characterization Tests
│  │  Once code is clean, write proper intention-revealing tests
│  │  The characterization tests served as scaffolding
│  └─ Now you can safely fix behavioral bugs you discovered
│
└─ Step 4: Fix Behavioral Issues (if any)
   Now that you have proper tests and clean code,
   fix any bugs discovered during characterization
   Each fix gets its own test + commit
```

### Writing Effective Characterization Tests

```python
# Strategy: Use the code itself to tell you what to assert

# 1. Call the function with representative inputs
result = process_order(sample_order)

# 2. Print the result
print(result)  # {'total': 108.25, 'tax': 8.25, 'status': 'pending'}

# 3. Assert on the printed output
def test_process_order_characterization():
    result = process_order(sample_order)
    assert result['total'] == 108.25
    assert result['tax'] == 8.25
    assert result['status'] == 'pending'

# 4. Cover edge cases the same way
def test_process_order_empty_items():
    result = process_order(Order(items=[]))
    # Even if this behavior is "wrong", capture it
    assert result['total'] == 0
    assert result['status'] == 'pending'  # maybe should be 'invalid'?
```

### When You Cannot Write Tests First

Sometimes characterization tests are impractical (tightly coupled UI, external service dependencies, time-based logic). Alternatives:

```
Cannot write characterization tests?
│
├─ Too coupled to test → Extract the testable parts first
│  Use "Sprout Method" or "Sprout Class":
│  1. Write the new logic in a new, testable function
│  2. Call it from the old code
│  3. Test the new function
│  4. Gradually move more logic into testable functions
│
├─ External service dependency → Record and replay
│  Use VCR/Polly/nock to record real responses
│  Replay them in tests
│
├─ UI-heavy → Snapshot/Approval tests
│  Capture screenshots or HTML output
│  Compare against approved baseline
│
└─ Time-based logic → Inject a clock
   Pass a clock/timer as a parameter
   Use a fake clock in tests
```

---

## Strangler Fig Pattern

For replacing a large legacy system or module incrementally, without a risky big-bang rewrite.

```
Strangler Fig Strategy
│
├─ Phase 1: Identify boundaries
│  │  Map the legacy system's entry points (API routes, function calls, events)
│  │  Each entry point is a candidate for strangling
│  └─ Prioritize by: risk (low first), value (high first), coupling (loose first)
│
├─ Phase 2: Build new implementation alongside old
│  │  New code lives in a new module/service
│  │  Both old and new exist simultaneously
│  └─ No modification to legacy code yet
│
├─ Phase 3: Route traffic to new implementation
│  │  Use a router/proxy/feature flag to direct requests
│  │  Start with a small percentage (canary)
│  │  Monitor for errors and performance differences
│  └─ Gradually increase percentage
│
├─ Phase 4: Remove legacy code
│  │  Once 100% traffic goes to new implementation
│  │  Keep legacy code for one release cycle (rollback safety)
│  └─ Then delete it
│
└─ Repeat for each entry point
```

### Example: Strangling a Legacy API Endpoint

```typescript
// Phase 2: New implementation alongside old
// old: /api/v1/users (legacy monolith)
// new: /api/v2/users (new service)

// Phase 3: Router decides which to call
app.get('/api/users', async (req, res) => {
  const useNewImplementation = await featureFlag('new-users-api', {
    userId: req.user?.id,
    percentage: 25,  // Start with 25% of traffic
  });

  if (useNewImplementation) {
    return newUsersService.getUsers(req, res);
  }
  return legacyUsersController.getUsers(req, res);
});

// Phase 4: Once at 100%, simplify
app.get('/api/users', (req, res) => newUsersService.getUsers(req, res));
```

---

## Parallel Change (Expand-Migrate-Contract)

For changing an interface without breaking consumers. Three phases: expand (add new), migrate (move consumers), contract (remove old).

```
Parallel Change Phases
│
├─ EXPAND: Add the new interface alongside the old
│  │  Both old and new work simultaneously
│  │  Old interface delegates to new implementation internally
│  └─ All existing tests continue to pass
│
├─ MIGRATE: Update all consumers to use the new interface
│  │  One consumer at a time
│  │  Each migration is a separate commit/PR
│  │  Old interface still works (backward compatible)
│  └─ Monitor for issues after each migration
│
└─ CONTRACT: Remove the old interface
   │  All consumers now use the new interface
   │  Delete old code and update tests
   └─ This is the only "breaking" change
```

### Example: Renaming a Function

```python
# EXPAND: Add new name, keep old as alias
def calculate_shipping_cost(order: Order) -> Money:
    """New name with improved logic."""
    # ... implementation ...

def calcShipping(order: Order) -> Money:
    """Deprecated: Use calculate_shipping_cost instead."""
    import warnings
    warnings.warn("calcShipping is deprecated, use calculate_shipping_cost", DeprecationWarning)
    return calculate_shipping_cost(order)

# MIGRATE: Update all call sites one by one
# grep for calcShipping, replace with calculate_shipping_cost
# Run tests after each file

# CONTRACT: Remove old function
# Delete calcShipping entirely
# Remove deprecation warning
```

### Example: Changing a Database Schema

```sql
-- EXPAND: Add new column alongside old
ALTER TABLE users ADD COLUMN full_name VARCHAR(255);

-- Application code writes to BOTH columns
-- UPDATE users SET full_name = first_name || ' ' || last_name, ...

-- MIGRATE: Backfill existing data
-- UPDATE users SET full_name = first_name || ' ' || last_name WHERE full_name IS NULL;
-- Update all queries to read from full_name

-- CONTRACT: Remove old columns
-- ALTER TABLE users DROP COLUMN first_name, DROP COLUMN last_name;
```

---

## Branch by Abstraction

For replacing an internal implementation without feature branches. Introduce an abstraction layer, swap the implementation behind it.

```
Branch by Abstraction
│
├─ Step 1: Create abstraction (interface/protocol/trait)
│  │  Define the contract that both old and new implementations satisfy
│  └─ All existing code uses the abstraction, not the concrete implementation
│
├─ Step 2: Wrap existing implementation
│  │  Make existing code implement the new abstraction
│  └─ All tests pass -- no behavior change
│
├─ Step 3: Build new implementation
│  │  New implementation also satisfies the abstraction
│  │  Test new implementation independently
│  └─ Old implementation is still the default
│
├─ Step 4: Switch
│  │  Change the wiring to use new implementation
│  │  Feature flag or config toggle for easy rollback
│  └─ Monitor in production
│
└─ Step 5: Clean up
   Remove old implementation
   Remove abstraction if only one implementation remains
   Remove feature flag
```

### Example:

```typescript
// Step 1: Define abstraction
interface PaymentGateway {
  charge(amount: Money, card: CardInfo): Promise<PaymentResult>;
  refund(paymentId: string, amount: Money): Promise<RefundResult>;
}

// Step 2: Wrap existing implementation
class StripeGateway implements PaymentGateway {
  async charge(amount: Money, card: CardInfo): Promise<PaymentResult> {
    // existing Stripe code, now behind the interface
  }
  async refund(paymentId: string, amount: Money): Promise<RefundResult> {
    // existing Stripe refund code
  }
}

// Step 3: Build new implementation
class SquareGateway implements PaymentGateway {
  async charge(amount: Money, card: CardInfo): Promise<PaymentResult> {
    // new Square implementation
  }
  async refund(paymentId: string, amount: Money): Promise<RefundResult> {
    // new Square refund implementation
  }
}

// Step 4: Switch via configuration
function createPaymentGateway(): PaymentGateway {
  if (config.paymentProvider === 'square') {
    return new SquareGateway();
  }
  return new StripeGateway(); // default/fallback
}
```

---

## Small Commits Strategy

Every commit during a refactoring must satisfy two invariants:

1. **Code compiles** (type-checks, no syntax errors)
2. **All tests pass** (no behavioral regressions)

### Commit Granularity Guide

```
Refactoring Commit Patterns
│
├─ Rename → 1 commit
│  "refactor: rename calcShipping to calculateShippingCost"
│
├─ Extract Function → 1 commit
│  "refactor: extract validateOrderItems from processOrder"
│
├─ Move File → 1 commit
│  "refactor: move utils/helpers.ts to lib/string-utils.ts"
│
├─ Extract Class → 2-3 commits
│  1. "refactor: extract PriceCalculator interface"
│  2. "refactor: implement PriceCalculator, delegate from OrderService"
│  3. "refactor: remove pricing logic from OrderService"
│
├─ Replace Algorithm → 2 commits
│  1. "test: add characterization tests for sorting"
│  2. "refactor: replace bubble sort with merge sort"
│
└─ Large Restructure → Many small commits
   Each file move or extraction is its own commit
   Never batch unrelated changes
```

### Git Workflow for Refactoring

```bash
# Start a refactoring session
git checkout -b refactor/extract-payment-service

# After each small refactoring step
git add -p  # Stage only the relevant changes
git commit -m "refactor: extract PaymentValidator from PaymentService"

# Verify at each step
npm test  # or pytest, cargo test, go test ./...

# If a step goes wrong, revert just that step
git revert HEAD

# When done, create a clean PR
# Each commit should be reviewable independently
```

---

## Feature Flags for Gradual Rollout

When a refactoring affects runtime behavior (e.g., new algorithm, new data flow), use feature flags to control rollout.

```
Feature Flag Strategy
│
├─ Before refactoring
│  │  Add a feature flag that defaults to OFF (old behavior)
│  └─ Deploy the flag infrastructure
│
├─ During refactoring
│  │  New code path guarded by the flag
│  │  Old code path remains the default
│  └─ Both paths are tested
│
├─ Rollout
│  │  Enable for internal users first
│  │  Enable for 1% → 10% → 50% → 100%
│  │  Monitor error rates, latency, correctness
│  └─ Rollback = disable the flag (instant, no deploy needed)
│
└─ Cleanup
   Remove the flag and old code path
   This is a separate PR after the rollout is complete
```

### Implementation Pattern

```typescript
// Simple feature flag check
async function searchProducts(query: string): Promise<Product[]> {
  if (await featureFlags.isEnabled('new-search-algorithm', { userId })) {
    return newSearchAlgorithm(query);
  }
  return legacySearch(query);
}
```

### Feature Flag Hygiene

| Rule | Why |
|------|-----|
| Remove flags within 2 sprints of 100% rollout | Stale flags accumulate and confuse |
| Name flags descriptively | `new-search-algorithm` not `flag-123` |
| Log flag evaluations | Debug which path was taken |
| Test both paths | Both old and new must have coverage |
| Flag owner documented | Someone must clean up the flag |

---

## Approval Testing / Snapshot Testing

Capture the output of existing code and use it as the test assertion. Ideal for characterization testing before refactoring.

### How It Works

```
Approval Testing Flow
│
├─ First run: Capture output → save as "approved" baseline
│  ├─ HTML output → screenshot or HTML snapshot
│  ├─ JSON output → save formatted JSON
│  ├─ Console output → save text
│  └─ API response → save response body
│
├─ Subsequent runs: Compare output against baseline
│  ├─ Match → test passes
│  └─ Mismatch → test fails, show diff
│     ├─ If expected change → approve new baseline
│     └─ If unexpected change → regression, investigate
│
└─ During refactoring: any output change is flagged
   You decide if the change is intentional or a bug
```

### Tools

| Language | Tool | Type |
|----------|------|------|
| JavaScript | Jest snapshots | `expect(result).toMatchSnapshot()` |
| JavaScript | Storybook Chromatic | Visual regression |
| Python | pytest-snapshot | `snapshot.assert_match(result)` |
| Python | Approval Tests | `verify(result)` |
| Go | go-snaps | `snaps.MatchSnapshot(t, result)` |
| Rust | insta | `insta::assert_snapshot!(result)` |
| Any | screenshot comparison | Playwright, Cypress, Percy |

### Jest Snapshot Example

```typescript
// Before refactoring: create baseline
test('renders user profile', () => {
  const { container } = render(<UserProfile user={mockUser} />);
  expect(container.innerHTML).toMatchSnapshot();
});

// During refactoring: any HTML change will fail this test
// If the change is intentional:
//   npx jest --updateSnapshot
```

### Python Approval Test Example

```python
from approvaltests import verify

def test_generate_report():
    report = generate_report(sample_data)
    verify(report)  # First run saves "approved" file
                    # Subsequent runs compare against it
```

---

## Rollback Strategies

```
Rollback Options (fastest to slowest)
│
├─ Feature flag toggle (seconds)
│  └─ Disable the flag → old code path runs instantly
│     No deployment needed
│
├─ Git revert (minutes)
│  └─ git revert <commit-hash>
│     Creates a new commit that undoes the change
│     Deploy the revert
│
├─ Redeploy previous version (minutes-hours)
│  └─ Roll back to previous container image / release tag
│     CI/CD pipeline handles the rest
│
├─ Database rollback (hours-days)
│  └─ If schema changed: run reverse migration
│     If data changed: restore from backup
│     Most disruptive, avoid if possible
│
└─ Cannot rollback (prevention only)
   Deleted data, sent emails, external API calls
   Design for forward-fix instead
```

### Forward-Fix vs Rollback Decision

```
Should you rollback or fix forward?
│
├─ Is the bug causing data loss or corruption?
│  └─ ROLLBACK immediately, fix later
│
├─ Is the bug affecting > 10% of users?
│  └─ ROLLBACK, then fix forward on a branch
│
├─ Is the fix obvious and small (< 5 lines)?
│  └─ FIX FORWARD with expedited review
│
├─ Is the bug cosmetic or low-severity?
│  └─ FIX FORWARD in next regular release
│
└─ Are you unsure of the scope?
   └─ ROLLBACK (when in doubt, be safe)
```

---

## Code Review Checklist for Refactoring PRs

```
Reviewer Checklist
│
├─ Behavior Preservation
│  [ ] No functional changes mixed with structural changes
│  [ ] Test suite passes (check CI, not just author's word)
│  [ ] Snapshot/approval tests show no unexpected diffs
│  [ ] Public API unchanged (or deprecated properly)
│
├─ Quality of Refactoring
│  [ ] Each commit is atomic and independently valid
│  [ ] Naming improves clarity (not just different)
│  [ ] Abstraction level is appropriate (not over-engineered)
│  [ ] No new duplication introduced
│  [ ] No circular dependencies introduced
│
├─ Safety
│  [ ] Characterization tests exist for changed code
│  [ ] Feature flag or rollback plan documented (if applicable)
│  [ ] Performance-sensitive code benchmarked before/after
│  [ ] No dead code left behind (old implementations removed)
│
└─ Completeness
   [ ] All references updated (imports, configs, docs, tests)
   [ ] Deprecation warnings added for public API changes
   [ ] Migration guide for downstream consumers (if applicable)
```

---

## Measuring Refactoring Success

Refactoring is an investment. Measure whether it paid off.

### Quantitative Metrics

| Metric | Before/After | Tool |
|--------|-------------|------|
| **Cyclomatic complexity** | Should decrease | radon, eslint, gocyclo |
| **Cognitive complexity** | Should decrease | SonarQube |
| **File length** | Should decrease (god files → smaller modules) | tokei, wc -l |
| **Test coverage** | Should increase or stay the same | coverage.py, istanbul, tarpaulin |
| **Build time** | Should not increase significantly | CI pipeline timing |
| **Bundle size** | Should not increase (may decrease with dead code removal) | webpack-bundle-analyzer |
| **Deployment frequency** | Should increase (easier to ship) | DORA metrics |
| **Change failure rate** | Should decrease (fewer regressions) | DORA metrics |

### Qualitative Indicators

| Signal | Meaning |
|--------|---------|
| Fewer merge conflicts in the area | Code is better organized, less contention |
| New features in the area are faster to build | Reduced coupling and clear boundaries |
| Fewer bug reports in the area | Cleaner code, better error handling |
| Team members are less afraid to change the code | Improved testability and readability |
| Code review comments shift from "I don't understand" to "looks good" | Better naming and structure |

### Before/After Comparison Template

```bash
# Capture BEFORE metrics
echo "=== BEFORE ==="
tokei src/module-to-refactor/          # Line counts
radon cc src/module-to-refactor/ -a    # Cyclomatic complexity (Python)
npx knip --reporter compact            # Unused code (JS/TS)

# ... do the refactoring ...

# Capture AFTER metrics
echo "=== AFTER ==="
tokei src/module-to-refactor/
radon cc src/module-to-refactor/ -a
npx knip --reporter compact

# Compare
# Complexity should go down
# Line count may go up slightly (more files, smaller each)
# Unused code count should go down
```

---

## Anti-Patterns in Refactoring Methodology

| Anti-pattern | Problem | Better Approach |
|--------------|---------|-----------------|
| Big-bang rewrite | High risk, nothing works for weeks | Strangler fig: replace incrementally |
| Refactoring without a goal | Endless polishing, no business value | Define success criteria before starting |
| Refactoring everything at once | Merge conflicts, hard to review, hard to rollback | One module at a time, one PR at a time |
| Skipping characterization tests | No safety net, cannot verify behavior preserved | Always capture current behavior first |
| Mixing refactoring with features | Cannot tell which caused a regression | Separate PRs: refactor first, then add feature |
| Not measuring improvement | Cannot justify the time investment | Capture before/after metrics |
| Stopping halfway | Half-old, half-new is worse than either | Plan for completion, or don't start |
| Over-designing for the future | YAGNI -- you are not going to need it | Refactor for today's needs, not hypothetical future |
| Refactoring shared library without coordinating consumers | Breaks downstream teams | Parallel change + deprecation period |
| No rollback plan | Stuck if something goes wrong in production | Always have a path back: feature flag, git revert, or previous deploy |