refactor

name	refactor
description	Surgical code refactoring to improve maintainability without changing behavior. Surfaces architectural friction and proposes deepening opportunities — turning shallow modules into deep ones. Covers deep vs shallow module analysis, deletion test, seam identification, extracting functions, breaking down god functions, improving type safety, and eliminating code smells. Use when user says "refactor", "clean up this code", "improve architecture", "make this more testable", or wants to find refactoring opportunities in a codebase.
license	MIT

Refactor

Improve code structure and readability without changing external behavior. The primary lens is depth: find shallow modules (interface nearly as complex as implementation) and deepen them (more capability behind a simpler interface).

When to Use

• Code is hard to understand or maintain
• Functions/classes are too large or too shallow
• Modules are tightly coupled across seams
• Adding features is difficult due to code structure
• The codebase needs to be more testable or AI-navigable
• User asks "clean up this code", "refactor this", "improve this", "improve architecture"

---

Vocabulary

Use these terms consistently in all suggestions:

• Module — anything with an interface and an implementation (function, class, package, slice). Scale-agnostic.
• Interface — everything a caller must know to use the module: types, invariants, error modes, ordering constraints, performance characteristics. Not just the type signature.
• Depth — leverage at the interface. A deep module provides a lot of behavior behind a small interface. A shallow module has an interface nearly as complex as its implementation.
• Seam — where an interface lives; a place behavior can be altered without editing in place.
• Adapter — a concrete thing satisfying an interface at a seam.
• Leverage — what callers get from depth: more capability per unit of interface learned.
• Locality — what maintainers get from depth: change, bugs, and knowledge concentrate in one place.

---

Process

1. Explore

Read the project's domain glossary (CONTEXT.md) and any ADRs in the area being touched, if they exist.

Then explore the codebase organically and note where you experience friction:

• Where does understanding one concept require bouncing between many small modules?
• Where are modules shallow — interface nearly as complex as the implementation?
• Where have pure functions been extracted just for testability, but the real bugs hide in how they're called (no locality)?
• Where do tightly-coupled modules leak across their seams?
• Which parts of the codebase are untested, or hard to test through their current interface?

Apply the deletion test to anything you suspect is shallow: imagine deleting the module. If complexity vanishes, it was a pass-through — the module wasn't earning its keep. If complexity reappears across N callers, the module was earning its keep.

2. Present Candidates

Present a numbered list of deepening opportunities. For each candidate:

• Files — which files/modules are involved
• Problem — why the current architecture is causing friction
• Solution — plain English description of what would change
• Benefits — explained in terms of locality and leverage, and also how tests would improve

If a candidate contradicts an existing ADR, only surface it when the friction is real enough to warrant revisiting. Mark it clearly.

Do NOT propose interfaces yet. Ask the user: "Which of these would you like to explore?"

3. Grilling Loop

Once the user picks a candidate, walk the design tree with them — constraints, dependencies, the shape of the deepened module, what sits behind the seam, what tests survive.

Side effects happen inline as decisions crystallize:

• Naming a module after a concept not in CONTEXT.md? Add the term. Create the file lazily if it doesn't exist.
• User rejects the candidate with a load-bearing reason? Offer an ADR to record the decision so future refactoring sessions don't re-suggest it.

4. Implement

Once the user approves a candidate:

1. Ensure tests exist for the current behavior (write them if missing)
2. Make one small change at a time
3. Run tests after each change
4. Commit at each safe state
5. Never mix refactoring with feature changes

---

Key Principles

The Deletion Test

The most powerful tool for identifying shallow modules. Imagine deleting the module entirely:

• If complexity vanishes → the module was a pass-through. Remove it or inline it.
• If complexity reappears across N callers → the module was earning its keep. Keep and potentially deepen it.

One Adapter = Hypothetical Seam. Two Adapters = Real Seam.

Don't introduce an abstraction boundary unless something actually varies across it. A UserRepositoryInterface with exactly one implementation and no test fake is premature indirection.

The Interface Is the Test Surface

Callers and tests cross the same seam. If you need to reach past the public interface to test something, the module is probably the wrong shape. Reshape the module so the test can exercise the real behavior through the public interface.

Depth Is a Property of the Interface, Not the Implementation

A deep module can be internally composed of small, testable parts — they just aren't part of the interface. A module can have internal seams (private to its implementation, used by its own tests) as well as the external seam at its interface.

---

Common Code Smells & Fixes

1. Long Method/Function

# BAD: 200-line function that does everything
- async function processOrder(orderId) {
-   // 50 lines: fetch order
-   // 30 lines: validate order
-   // 40 lines: calculate pricing
-   // 30 lines: update inventory
-   // 20 lines: create shipment
-   // 30 lines: send notifications
- }

# GOOD: Broken into focused functions
+ async function processOrder(orderId) {
+   const order = await fetchOrder(orderId);
+   validateOrder(order);
+   const pricing = calculatePricing(order);
+   await updateInventory(order);
+   const shipment = await createShipment(order);
+   await sendNotifications(order, pricing, shipment);
+   return { order, pricing, shipment };
+ }

2. Duplicated Code

# BAD: Same logic in multiple places
- function calculateUserDiscount(user) {
-   if (user.membership === 'gold') return user.total * 0.2;
-   if (user.membership === 'silver') return user.total * 0.1;
-   return 0;
- }
-
- function calculateOrderDiscount(order) {
-   if (order.user.membership === 'gold') return order.total * 0.2;
-   if (order.user.membership === 'silver') return order.total * 0.1;
-   return 0;
- }

# GOOD: Extract common logic
+ function getMembershipDiscountRate(membership) {
+   const rates = { gold: 0.2, silver: 0.1 };
+   return rates[membership] || 0;
+ }
+
+ function calculateUserDiscount(user) {
+   return user.total * getMembershipDiscountRate(user.membership);
+ }
+
+ function calculateOrderDiscount(order) {
+   return order.total * getMembershipDiscountRate(order.user.membership);
+ }

3. Large Class/Module

# BAD: God object that knows too much
- class UserManager {
-   createUser() { /* ... */ }
-   updateUser() { /* ... */ }
-   deleteUser() { /* ... */ }
-   sendEmail() { /* ... */ }
-   generateReport() { /* ... */ }
-   handlePayment() { /* ... */ }
-   validateAddress() { /* ... */ }
-   // 50 more methods...
- }

# GOOD: Single responsibility per class
+ class UserService {
+   create(data) { /* ... */ }
+   update(id, data) { /* ... */ }
+   delete(id) { /* ... */ }
+ }
+
+ class EmailService {
+   send(to, subject, body) { /* ... */ }
+ }
+
+ class ReportService {
+   generate(type, params) { /* ... */ }
+ }
+
+ class PaymentService {
+   process(amount, method) { /* ... */ }
+ }

4. Long Parameter List

# BAD: Too many parameters
- function createUser(email, password, name, age, address, city, country, phone) {
-   /* ... */
- }

# GOOD: Group related parameters
+ interface UserData {
+   email: string;
+   password: string;
+   name: string;
+   age?: number;
+   address?: Address;
+   phone?: string;
+ }
+
+ function createUser(data: UserData) {
+   /* ... */
+ }

# EVEN BETTER: Use builder pattern for complex construction
+ const user = UserBuilder
+   .email('[e-mail]')
+   .password('secure123')
+   .name('Test User')
+   .address(address)
+   .build();

5. Feature Envy

# BAD: Method that uses another object's data more than its own
- class Order {
-   calculateDiscount(user) {
-     if (user.membershipLevel === 'gold') {
+       return this.total * 0.2;
+     }
+     if (user.accountAge > 365) {
+       return this.total * 0.1;
+     }
+     return 0;
+   }
+ }

# GOOD: Move logic to the object that owns the data
+ class User {
+   getDiscountRate(orderTotal) {
+     if (this.membershipLevel === 'gold') return 0.2;
+     if (this.accountAge > 365) return 0.1;
+     return 0;
+   }
+ }
+
+ class Order {
+   calculateDiscount(user) {
+     return this.total * user.getDiscountRate(this.total);
+   }
+ }

6. Primitive Obsession

# BAD: Using primitives for domain concepts
- function sendEmail(to, subject, body) { /* ... */ }
- sendEmail('[e-mail]', 'Hello', '...');

- function createPhone(country, number) {
-   return `${country}-${number}`;
- }

# GOOD: Use domain types
+ class Email {
+   private constructor(public readonly value: string) {
+     if (!Email.isValid(value)) throw new Error('Invalid email');
+   }
+   static create(value: string) { return new Email(value); }
+   static isValid(email: string) { return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email); }
+ }
+
+ class PhoneNumber {
+   constructor(
+     public readonly country: string,
+     public readonly number: string
+   ) {
+     if (!PhoneNumber.isValid(country, number)) throw new Error('Invalid phone');
+   }
+   toString() { return `${this.country}-${this.number}`; }
+   static isValid(country: string, number: string) { /* ... */ }
+ }
+
+ // Usage
+ const email = Email.create('[e-mail]');
+ const phone = new PhoneNumber('1', '555-1234');

7. Magic Numbers/Strings

# BAD: Unexplained values
- if (user.status === 2) { /* ... */ }
- const discount = total * 0.15;
- setTimeout(callback, 86400000);

# GOOD: Named constants
+ const UserStatus = {
+   ACTIVE: 1,
+   INACTIVE: 2,
+   SUSPENDED: 3
+ } as const;
+
+ const DISCOUNT_RATES = {
+   STANDARD: 0.1,
+   PREMIUM: 0.15,
+   VIP: 0.2
+ } as const;
+
+ const ONE_DAY_MS = 24 * 60 * 60 * 1000;
+
+ if (user.status === UserStatus.INACTIVE) { /* ... */ }
+ const discount = total * DISCOUNT_RATES.PREMIUM;
+ setTimeout(callback, ONE_DAY_MS);

8. Nested Conditionals

# BAD: Arrow code
- function process(order) {
-   if (order) {
-     if (order.user) {
-       if (order.user.isActive) {
-         if (order.total > 0) {
-           return processOrder(order);
+         } else {
+           return { error: 'Invalid total' };
+         }
+       } else {
+         return { error: 'User inactive' };
+       }
+     } else {
+       return { error: 'No user' };
+     }
+   } else {
+     return { error: 'No order' };
+   }
+ }

# GOOD: Guard clauses / early returns
+ function process(order) {
+   if (!order) return { error: 'No order' };
+   if (!order.user) return { error: 'No user' };
+   if (!order.user.isActive) return { error: 'User inactive' };
+   if (order.total <= 0) return { error: 'Invalid total' };
+   return processOrder(order);
+ }

# EVEN BETTER: Using Result type
+ function process(order): Result<ProcessedOrder, Error> {
+   return Result.combine([
+     validateOrderExists(order),
+     validateUserExists(order),
+     validateUserActive(order.user),
+     validateOrderTotal(order)
+   ]).flatMap(() => processOrder(order));
+ }

9. Dead Code

# BAD: Unused code lingers
- function oldImplementation() { /* ... */ }
- const DEPRECATED_VALUE = 5;
- import { unusedThing } from './somewhere';
- // Commented out code
- // function oldCode() { /* ... */ }

# GOOD: Remove it
+ // Delete unused functions, imports, and commented code
+ // If you need it again, git history has it

10. Inappropriate Intimacy

# BAD: One class reaches deep into another
- class OrderProcessor {
-   process(order) {
-     order.user.profile.address.street;  // Too intimate
-     order.repository.connection.config;  // Breaking encapsulation
+   }
+ }

# GOOD: Ask, don't tell
+ class OrderProcessor {
+   process(order) {
+     order.getShippingAddress();  // Order knows how to get it
+     order.save();  // Order knows how to save itself
+   }
+ }

---

Extract Method Refactoring

Before and After

# Before: One long function
- function printReport(users) {
-   console.log('USER REPORT');
-   console.log('============');
-   console.log('');
-   console.log(`Total users: ${users.length}`);
-   console.log('');
-   console.log('ACTIVE USERS');
-   console.log('------------');
-   const active = users.filter(u => u.isActive);
-   active.forEach(u => {
-     console.log(`- ${u.name} (${u.email})`);
-   });
-   console.log('');
-   console.log(`Active: ${active.length}`);
-   console.log('');
-   console.log('INACTIVE USERS');
-   console.log('--------------');
-   const inactive = users.filter(u => !u.isActive);
-   inactive.forEach(u => {
-     console.log(`- ${u.name} (${u.email})`);
-   });
-   console.log('');
-   console.log(`Inactive: ${inactive.length}`);
- }

# After: Extracted methods
+ function printReport(users) {
+   printHeader('USER REPORT');
+   console.log(`Total users: ${users.length}\n`);
+   printUserSection('ACTIVE USERS', users.filter(u => u.isActive));
+   printUserSection('INACTIVE USERS', users.filter(u => !u.isActive));
+ }
+
+ function printHeader(title) {
+   const line = '='.repeat(title.length);
+   console.log(title);
+   console.log(line);
+   console.log('');
+ }
+
+ function printUserSection(title, users) {
+   console.log(title);
+   console.log('-'.repeat(title.length));
+   users.forEach(u => console.log(`- ${u.name} (${u.email})`));
+   console.log('');
+   console.log(`${title.split(' ')[0]}: ${users.length}`);
+   console.log('');
+ }

---

Introducing Type Safety

From Untyped to Typed

# Before: No types
- function calculateDiscount(user, total, membership, date) {
-   if (membership === 'gold' && date.getDay() === 5) {
-     return total * 0.25;
-   }
-   if (membership === 'gold') return total * 0.2;
-   return total * 0.1;
- }

# After: Full type safety
+ type Membership = 'bronze' | 'silver' | 'gold';
+
+ interface User {
+   id: string;
+   name: string;
+   membership: Membership;
+ }
+
+ interface DiscountResult {
+   original: number;
+   discount: number;
+   final: number;
+   rate: number;
+ }
+
+ function calculateDiscount(
+   user: User,
+   total: number,
+   date: Date = new Date()
+ ): DiscountResult {
+   if (total < 0) throw new Error('Total cannot be negative');
+
+   let rate = 0.1; // Default bronze
+
+   if (user.membership === 'gold' && date.getDay() === 5) {
+     rate = 0.25; // Friday bonus for gold
+   } else if (user.membership === 'gold') {
+     rate = 0.2;
+   } else if (user.membership === 'silver') {
+     rate = 0.15;
+   }
+
+   const discount = total * rate;
+
+   return {
+     original: total,
+     discount,
+     final: total - discount,
+     rate
+   };
+ }

---

Design Patterns for Refactoring

Strategy Pattern

# Before: Conditional logic
- function calculateShipping(order, method) {
-   if (method === 'standard') {
-     return order.total > 50 ? 0 : 5.99;
-   } else if (method === 'express') {
-     return order.total > 100 ? 9.99 : 14.99;
+   } else if (method === 'overnight') {
+     return 29.99;
+   }
+ }

# After: Strategy pattern
+ interface ShippingStrategy {
+   calculate(order: Order): number;
+ }
+
+ class StandardShipping implements ShippingStrategy {
+   calculate(order: Order) {
+     return order.total > 50 ? 0 : 5.99;
+   }
+ }
+
+ class ExpressShipping implements ShippingStrategy {
+   calculate(order: Order) {
+     return order.total > 100 ? 9.99 : 14.99;
+   }
+ }
+
+ class OvernightShipping implements ShippingStrategy {
+   calculate(order: Order) {
+     return 29.99;
+   }
+ }
+
+ function calculateShipping(order: Order, strategy: ShippingStrategy) {
+   return strategy.calculate(order);
+ }

Chain of Responsibility

# Before: Nested validation
- function validate(user) {
-   const errors = [];
-   if (!user.email) errors.push('Email required');
+   else if (!isValidEmail(user.email)) errors.push('Invalid email');
+   if (!user.name) errors.push('Name required');
+   if (user.age < 18) errors.push('Must be 18+');
+   if (user.country === 'blocked') errors.push('Country not supported');
+   return errors;
+ }

# After: Chain of responsibility
+ abstract class Validator {
+   abstract validate(user: User): string | null;
+   setNext(validator: Validator): Validator {
+     this.next = validator;
+     return validator;
+   }
+   validate(user: User): string | null {
+     const error = this.doValidate(user);
+     if (error) return error;
+     return this.next?.validate(user) ?? null;
+   }
+ }
+
+ class EmailRequiredValidator extends Validator {
+   doValidate(user: User) {
+     return !user.email ? 'Email required' : null;
+   }
+ }
+
+ class EmailFormatValidator extends Validator {
+   doValidate(user: User) {
+     return user.email && !isValidEmail(user.email) ? 'Invalid email' : null;
+   }
+ }
+
+ // Build the chain
+ const validator = new EmailRequiredValidator()
+   .setNext(new EmailFormatValidator())
+   .setNext(new NameRequiredValidator())
+   .setNext(new AgeValidator())
+   .setNext(new CountryValidator());

---

Refactoring Checklist

Depth & Architecture

• Shallow modules identified (deletion test applied)
• Seams are real (two+ adapters, not hypothetical)
• Tests exercise behavior through public interfaces
• Related code lives together (high cohesion)
• Dependencies flow in one direction
• No circular dependencies

Code Quality

• Functions are focused (one behavior)
• No duplicated logic across distant modules
• Descriptive names using project domain language
• No magic numbers/strings
• Dead code removed

Type Safety

• Types defined for all public APIs
• No any types without justification
• Nullable types explicitly marked

Process

• Tests existed before refactoring began
• Each change was small and tested
• Behavior is unchanged
• All tests pass

---

Common Refactoring Operations

Operation	Description
Extract Method	Turn code fragment into method
Extract Class	Move behavior to new class
Extract Interface	Create interface from implementation
Inline Method	Move method body back to caller
Inline Class	Move class behavior to caller
Pull Up Method	Move method to superclass
Push Down Method	Move method to subclass
Rename Method/Variable	Improve clarity
Introduce Parameter Object	Group related parameters
Replace Conditional with Polymorphism	Use polymorphism instead of switch/if
Replace Magic Number with Constant	Named constants
Decompose Conditional	Break complex conditions
Consolidate Conditional	Combine duplicate conditions
Replace Nested Conditional with Guard Clauses	Early returns
Introduce Null Object	Eliminate null checks
Replace Type Code with Class/Enum	Strong typing
Replace Inheritance with Delegation	Composition over inheritance