Refactor Overview

Improve code structure and readability without changing external behavior. Refactoring is gradual evolution, not revolution. Use this for improving existing code, not rewriting from scratch.

When to Use

Use this skill when:

Code is hard to understand or maintain Functions/classes are too large Code smells need addressing Adding features is difficult due to code structure User asks "clean up this code", "refactor this", "improve this" Refactoring Principles The Golden Rules Behavior is preserved - Refactoring doesn't change what the code does, only how Small steps - Make tiny changes, test after each Version control is your friend - Commit before and after each safe state Tests are essential - Without tests, you're not refactoring, you're editing One thing at a time - Don't mix refactoring with feature changes When NOT to Refactor - Code that works and won't change again (if it ain't broke...) - Critical production code without tests (add tests first) - When you're under a tight deadline - "Just because" - need a clear purpose

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 };
}
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);
}
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) { / ... / }
}
Long Parameter List

BAD: Too many parameters

function createUser(email, password, name, age, address, city, country, phone) {
/ ... /
}

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('test@example.com')
.password('secure123')
.name('Test User')
.address(address)
.build();
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);
}
}
Primitive Obsession

BAD: Using primitives for domain concepts

function sendEmail(to, subject, body) { / ... / }
sendEmail('user@example.com', '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('user@example.com');
const phone = new PhoneNumber('1', '555-1234');
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);
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 BETER: Using Result type

function process(order): Result {
return Result.combine([
validateOrderExists(order),
validateUserExists(order),
validateUserActive(order.user),
validateOrderTotal(order)
]).flatMap(() => processOrder(order));
}
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
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 Steps Safe Refactoring Process 1. PREPARE - Ensure tests exist (write them if missing) - Commit current state - Create feature branch

IDENTIFY
Find the code smell to address
Understand what the code does
Plan the refactoring
REFACTOR (small steps)
Make one small change
Run tests
Commit if tests pass
Repeat
VERIFY
All tests pass
Manual testing if needed
Performance unchanged or improved
CLEAN UP
Update comments
Update documentation
Final commit

Refactoring Checklist Code Quality Functions are small (< 50 lines) Functions do one thing No duplicated code Descriptive names (variables, functions, classes) No magic numbers/strings Dead code removed Structure Related code is together Clear module boundaries Dependencies flow in one direction No circular dependencies Type Safety Types defined for all public APIs No any types without justification Nullable types explicitly marked Testing Refactored code is tested Tests cover edge cases 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

安装