refactoring

name	refactoring
description	Use when user explicitly asks to refactor code, or when test coverage is requested for untested code with side effects. Enforces Functional Core Imperative Shell pattern extraction before any changes.

Refactoring with Functional Core, Imperative Shell

Extract pure functions before refactoring. Never refactor code that mixes logic with side effects.

When to Use

• User asks to "refactor", "consolidate", "extract", or "reduce duplication"
• Test coverage requested for hooks, components, or code with I/O
• Code review reveals logic buried in side-effect-heavy code

Core Pattern

1. ANALYZE → Identify pure logic vs side effects
2. EXTRACT → Move pure logic to utils/ as pure functions
3. TEST → Write output-based tests for extracted functions
4. REFACTOR → Modify imperative shell (now thin wrapper)

Implementation

Step 1: Analyze Before Touching Code

// IDENTIFY in existing code:
// - Pure logic (calculations, transformations, validations)
// - Side effects (API calls, state updates, localStorage, Date.now())

// Example: useCommentSuggestions.ts
// PURE: isCacheValid(entry, currentTime, ttl) → boolean
// IMPURE: localStorage.getItem(), Date.now(), queryClient.setQueryData()

Step 2: Extract Pure Functions

// Before: Logic mixed with side effects
const loadFromCache = (key: string) => {
  const cached = localStorage.getItem(key);  // side effect
  if (!cached) return undefined;
  const entry = JSON.parse(cached);
  return Date.now() - entry.timestamp <= TTL ? entry.data : undefined;  // impure
};

// After: Pure function extracted
export const isCacheValid = (timestamp: number, currentTime: number, ttl: number): boolean =>
  currentTime - timestamp <= ttl;

// Imperative shell becomes thin
const loadFromCache = (key: string) => {
  const cached = localStorage.getItem(key);
  if (!cached) return undefined;
  const entry = JSON.parse(cached);
  return isCacheValid(entry.timestamp, Date.now(), TTL) ? entry.data : undefined;
};

Step 3: Test Only Pure Functions

describe('isCacheValid', () => {
  it('returns true when within TTL', () => {
    expect(isCacheValid(1000, 2000, 5000)).toBe(true);
  });

  it('returns false when TTL exceeded', () => {
    expect(isCacheValid(1000, 10000, 5000)).toBe(false);
  });
});

Common Mistakes

Mistake	Why It's Wrong	Do Instead
Refactor first, test later	No safety net for regressions	Extract + test pure functions first
Mock Date.now() in tests	Testing implementation, not behavior	Inject time as parameter
Test hooks directly	Requires QueryClient, context mocking	Extract logic, test pure functions
Skip analysis step	Miss extraction opportunities	Always analyze pure vs impure first

Red Flags

Stop and re-analyze if you find yourself:

• Writing vi.mock() for more than external APIs
• Testing a function that calls useState, useQuery, or Firebase
• Unable to test without renderHook() or QueryClientProvider