| name | clean-code |
| model | standard |
| category | testing |
| description | Pragmatic coding standards for writing clean, maintainable code — naming, functions, structure, anti-patterns, and pre-edit safety checks. Use when writing new code, refactoring existing code, reviewing code quality, or establishing coding standards. |
| version | 2 |
Clean Code
Be concise, direct, and solution-focused. Clean code reads like well-written prose — every name reveals intent, every function does one thing, and every abstraction earns its place.
Installation
OpenClaw / Moltbot / Clawbot
npx clawhub@latest install clean-code
Core Principles
| Principle | Rule | Practical Test |
|---|
| SRP | Single Responsibility — each function/class does ONE thing | "Can I describe what this does without using 'and'?" |
| DRY | Don't Repeat Yourself — extract duplicates, reuse | "Have I written this logic before?" |
| KISS | Keep It Simple — simplest solution that works | "Is there a simpler way to achieve this?" |
| YAGNI | You Aren't Gonna Need It — don't build unused features | "Does anyone need this right now?" |
| Boy Scout | Leave code cleaner than you found it | "Is this file better after my change?" |
Naming Rules
Names are the most important documentation. A good name eliminates the need for a comment.
| Element | Convention | Bad | Good |
|---|
| Variables | Reveal intent | n, d, tmp | userCount, elapsed, activeUsers |
| Functions | Verb + noun | user(), calc() | getUserById(), calculateTotal() |
| Booleans | Question form | active, flag | isActive, hasPermission, canEdit |
| Constants | SCREAMING_SNAKE | max, timeout | MAX_RETRY_COUNT, REQUEST_TIMEOUT_MS |
| Classes | Noun, singular | Manager, Data | UserRepository, OrderService |
| Enums | PascalCase values | 'pending' string | Status.Pending |
Rule: If you need a comment to explain a name, rename it.
Naming Anti-Patterns
| Anti-Pattern | Problem | Fix |
|---|
Cryptic abbreviations (usrMgr, cfg) | Unreadable in 6 months | Spell it out — IDE autocomplete makes long names free |
Generic names (data, info, item, handler) | Says nothing about purpose | Use domain-specific names that reveal intent |
Misleading names (getUserList returns one user) | Actively deceives readers | Match name to behavior, or change the behavior |
Hungarian notation (strName, nCount, IUser) | Redundant with type system | Let TypeScript/IDE show types; names describe purpose |
Function Rules
| Rule | Guideline | Why |
|---|
| Small | Max 20 lines, ideally 5-10 | Fits in your head |
| One Thing | Does one thing, does it well | Testable and nameable |
| One Level | One level of abstraction per function | Readable top to bottom |
| Few Args | Max 3 arguments, prefer 0-2 | Easy to call correctly |
| No Side Effects | Don't mutate inputs unexpectedly | Predictable behavior |
Guard Clauses
Flatten nested conditionals with early returns. Never nest deeper than 2 levels.
function processOrder(order: Order) {
if (order) {
if (order.items.length > 0) {
if (order.customer) {
if (order.customer.isVerified) {
return submitOrder(order);
}
}
}
}
throw new Error('Invalid order');
}
function processOrder(order: Order) {
if (!order) throw new Error('No order');
if (!order.items.length) throw new Error('No items');
if (!order.customer) throw new Error('No customer');
if (!order.customer.isVerified) throw new Error('Customer not verified');
return submitOrder(order);
}
Parameter Objects
When a function needs more than 3 arguments, use an options object.
createUser('John', 'Doe', 'john@example.com', 'secret', 'admin', 'Engineering');
createUser({
firstName: 'John',
lastName: 'Doe',
email: 'john@example.com',
password: 'secret',
role: 'admin',
department: 'Engineering',
});
Code Structure Patterns
| Pattern | When to Apply | Benefit |
|---|
| Guard Clauses | Edge cases at function start | Flat, readable flow |
| Flat > Nested | Any nesting beyond 2 levels | Reduced cognitive load |
| Composition | Complex operations | Small, testable pieces |
| Colocation | Related code across files | Easier to find and change |
| Extract Function | Comments separating "sections" | Self-documenting code |
Composition Over God Functions
async function processOrder(order: Order) {
return { success: true };
}
async function processOrder(order: Order) {
validateOrder(order);
const totals = calculateOrderTotals(order);
const payment = await processPayment(order.customer, totals);
await sendOrderConfirmation(order, payment);
await updateInventory(order.items);
return { success: true, orderId: payment.orderId };
}
Return Type Consistency
Functions should return consistent types. Use discriminated unions for multiple outcomes.
function getUser(id: string) {
const user = database.find(id);
if (!user) return false;
if (user.isDeleted) return null;
return user;
}
type GetUserResult =
| { status: 'found'; user: User }
| { status: 'not_found' }
| { status: 'deleted' };
function getUser(id: string): GetUserResult {
const user = database.find(id);
if (!user) return { status: 'not_found' };
if (user.isDeleted) return { status: 'deleted' };
return { status: 'found', user };
}
Anti-Patterns
| Anti-Pattern | Problem | Fix |
|---|
| Comment every line | Noise obscures signal | Delete obvious comments; comment why, not what |
| Helper for one-liner | Unnecessary indirection | Inline the code |
| Factory for 2 objects | Over-engineering | Direct instantiation |
utils.ts with 1 function | Junk drawer file | Put code where it's used |
| Deep nesting | Unreadable flow | Guard clauses and early returns |
| Magic numbers | Unclear intent | Named constants |
| God functions | Untestable, unreadable | Split by responsibility |
| Commented-out code | Dead code confusion | Delete it; git remembers |
| TODO sprawl | Never gets done | Track in issue tracker, not code |
| Premature abstraction | Wrong abstraction is worse than none | Wait for 3+ duplicates before abstracting |
| Copy-paste programming | Duplicated bugs | Extract shared logic |
| Exception-driven control flow | Slow and confusing | Use explicit conditionals |
| Stringly-typed code | Typos and missed cases | Use enums or union types |
| Callback hell | Pyramid of doom | Use async/await |
Pre-Edit Safety Check
Before changing any file, answer these questions to avoid cascading breakage:
| Question | Why |
|---|
| What imports this file? | Dependents might break on interface changes |
| What does this file import? | You might need to update the contract |
| What tests cover this? | Tests might fail — update them alongside code |
| Is this a shared component? | Multiple consumers means wider blast radius |
File to edit: UserService.ts
├── Who imports this? → UserController.ts, AuthController.ts
├── Do they need changes too? → Check function signatures
└── What tests cover this? → UserService.test.ts
Rule: Edit the file + all dependent files in the SAME task. Never leave broken imports or missing updates.
Self-Check Before Completing
Before marking any task complete, verify:
| Check | Question |
|---|
| Goal met? | Did I do exactly what was asked? |
| Files edited? | Did I modify all necessary files, including dependents? |
| Code works? | Did I verify the change compiles and runs? |
| No errors? | Do lint and type checks pass? |
| Nothing forgotten? | Any edge cases or dependent files missed? |
NEVER Do
- NEVER add comments that restate the code — if the code needs a comment to explain what it does, rename things until it doesn't
- NEVER create abstractions for fewer than 3 use cases — premature abstraction is worse than duplication
- NEVER leave commented-out code in the codebase — delete it; version control exists for history
- NEVER write functions longer than 20 lines — extract sub-functions until each does one thing
- NEVER nest deeper than 2 levels — use guard clauses, early returns, or extract functions
- NEVER use magic numbers or strings — define named constants with clear semantics
- NEVER edit a file without checking what depends on it — broken imports and missing updates are the most common source of bugs in multi-file changes
- NEVER leave a task with failing lint or type checks — fix all errors before marking complete
References
Detailed guides for specific clean code topics:
| Reference | Description |
|---|
| Anti-Patterns | 21 common mistakes with bad/good code examples across naming, functions, structure, and comments |
| Code Smells | Classic code smells catalog with detection patterns — Bloaters, OO Abusers, Change Preventers, Dispensables, Couplers |
| Refactoring Catalog | Essential refactoring patterns with before/after examples and step-by-step mechanics |
Anti-Patterns Gallery
Common coding mistakes with explanations and fixes. Each pattern includes bad/good examples to make code review and refactoring actionable.
Naming Anti-Patterns
1. Cryptic Abbreviations
Problem: Abbreviations save keystrokes but cost readability. Future readers (including yourself) won't remember what usrMgr means.
const usrMgr = new UsrMgr();
const cfg = loadCfg();
const btn = document.getElementById('sbmt');
const val = calc(x, y, z);
const userManager = new UserManager();
const config = loadConfig();
const submitButton = document.getElementById('submit');
const totalPrice = calculateTotalPrice(quantity, unitPrice, taxRate);
Rule: Spell it out. IDE autocomplete makes long names free.
2. Generic Names
Problem: Names like data, info, item, thing, manager, handler tell you nothing about what the code does.
function processData(data: any) {
const info = getData();
const result = handle(info);
return result;
}
class Manager {
items: any[] = [];
process() { }
}
function validateUserRegistration(registration: UserRegistration) {
const existingUser = findUserByEmail(registration.email);
const validationResult = checkEmailAvailability(existingUser);
return validationResult;
}
class ShoppingCart {
lineItems: CartLineItem[] = [];
calculateTotal() { }
}
Rule: Names should reveal intent. Ask "what does this actually do?"
3. Misleading Names
Problem: Names that lie are worse than bad names. They actively deceive readers.
function getUserList() {
return this.currentUser;
}
const isValid = checkDate(date);
class AccountList extends Map { }
function getCurrentUser() {
return this.currentUser;
}
const normalizedDate = normalizeDate(date);
class AccountRegistry extends Map { }
Rule: If the name doesn't match the behavior, change the name (or the behavior).
4. Hungarian Notation
Problem: Encoding types in names was useful in weakly-typed languages. TypeScript makes it redundant and noisy.
const strName: string = 'Alice';
const nCount: number = 42;
const arrUsers: User[] = [];
const bIsActive: boolean = true;
interface IUser { }
type TUserRole = 'admin' | 'user';
const name: string = 'Alice';
const count: number = 42;
const users: User[] = [];
const isActive: boolean = true;
interface User { }
type UserRole = 'admin' | 'user';
Rule: Let the type system handle types. Names should describe purpose.
Function Anti-Patterns
5. God Functions
Problem: Functions over 20 lines are hard to understand, test, and modify. They usually do too many things.
async function processOrder(order: Order) {
if (!order.items.length) throw new Error('Empty order');
if (!order.customer) throw new Error('No customer');
let subtotal = 0;
for (const item of order.items) {
subtotal += item.price * item.quantity;
}
const paymentResult = await stripe.charge();
await sendEmail();
await sendSMS();
return { success: true };
}
async function processOrder(order: Order) {
validateOrder(order);
const totals = calculateOrderTotals(order);
const payment = await processPayment(order.customer, totals);
await sendOrderConfirmation(order, payment);
await updateInventory(order.items);
return { success: true, orderId: payment.orderId };
}
function validateOrder(order: Order): void {
if (!order.items.length) throw new Error('Empty order');
if (!order.customer) throw new Error('No customer');
}
function calculateOrderTotals(order: Order): OrderTotals {
const subtotal = order.items.reduce(
(sum, item) => sum + item.price * item.quantity, 0
);
return { subtotal, tax: subtotal * 0.1, total: subtotal * 1.1 };
}
Rule: Extract until each function does exactly one thing.
6. Too Many Parameters
Problem: Functions with 4+ parameters are hard to call correctly and often indicate the function does too much.
function createUser(
firstName: string,
lastName: string,
email: string,
password: string,
role: string,
department: string,
startDate: Date,
managerId: string | null,
isActive: boolean
) {
}
createUser('John', 'Doe', 'john@example.com', 'secret', 'admin', 'Engineering', new Date(), null, true);
interface CreateUserOptions {
firstName: string;
lastName: string;
email: string;
password: string;
role: UserRole;
department: string;
startDate?: Date;
managerId?: string;
isActive?: boolean;
}
function createUser(options: CreateUserOptions) {
const { firstName, lastName, email, role, isActive = true } = options;
}
createUser({
firstName: 'John',
lastName: 'Doe',
email: 'john@example.com',
password: 'secret',
role: 'admin',
department: 'Engineering',
});
Rule: More than 3 parameters? Use an options object.
7. Boolean Flag Parameters
Problem: Boolean parameters hide branching logic and make function calls unreadable.
renderButton('Submit', true, false, true);
function renderButton(
label: string,
isPrimary: boolean,
isDisabled: boolean,
isLoading: boolean
) {
}
renderButton({
label: 'Submit',
variant: 'primary',
state: 'loading',
});
renderPrimaryButton('Submit');
renderLoadingButton('Submit');
type ButtonVariant = 'primary' | 'secondary' | 'ghost';
type ButtonState = 'default' | 'loading' | 'disabled';
Rule: Boolean parameters should be in an options object with named properties.
8. Side Effects in Getters
Problem: Getters that modify state violate the principle of least surprise. Readers expect getters to be pure.
class ShoppingCart {
private _items: CartItem[] = [];
private _lastAccessed: Date;
get items() {
this._lastAccessed = new Date();
this.logAccess();
return this._items;
}
get totalPrice() {
this.recalculateDiscounts();
return this._items.reduce((sum, i) => sum + i.price, 0);
}
}
class ShoppingCart {
private _items: CartItem[] = [];
private _lastAccessed: Date;
get items() {
return this._items;
}
get totalPrice() {
return this._items.reduce((sum, i) => sum + i.price, 0);
}
recordAccess() {
this._lastAccessed = new Date();
this.logAccess();
}
applyDiscounts() {
this.recalculateDiscounts();
}
}
Rule: Getters should be pure. Make side effects explicit with verbs.
9. Returning Different Types
Problem: Functions that return different types based on conditions make code unpredictable and hard to type.
function getUser(id: string) {
const user = database.find(id);
if (!user) {
return false;
}
if (user.isDeleted) {
return null;
}
return user;
}
const result = getUser('123');
if (result === false) { }
else if (result === null) { }
else { }
type GetUserResult =
| { status: 'found'; user: User }
| { status: 'not_found' }
| { status: 'deleted' };
function getUser(id: string): GetUserResult {
const user = database.find(id);
if (!user) {
return { status: 'not_found' };
}
if (user.isDeleted) {
return { status: 'deleted' };
}
return { status: 'found', user };
}
const result = getUser('123');
if (result.status === 'found') {
console.log(result.user.name);
}
Rule: Return consistent types. Use discriminated unions for multiple outcomes.
Structure Anti-Patterns
10. Deep Nesting (Pyramid of Doom)
Problem: Deeply nested code is hard to follow and usually indicates missing abstractions.
function processOrder(order: Order) {
if (order) {
if (order.items.length > 0) {
if (order.customer) {
if (order.customer.isVerified) {
if (order.paymentMethod) {
return submitOrder(order);
} else {
throw new Error('No payment method');
}
} else {
throw new Error('Customer not verified');
}
} else {
throw new Error('No customer');
}
} else {
throw new Error('No items');
}
} else {
throw new Error('No order');
}
}
function processOrder(order: Order) {
if (!order) throw new Error('No order');
if (!order.items.length) throw new Error('No items');
if (!order.customer) throw new Error('No customer');
if (!order.customer.isVerified) throw new Error('Customer not verified');
if (!order.paymentMethod) throw new Error('No payment method');
return submitOrder(order);
}
Rule: Use guard clauses for early returns. Max 2 levels of nesting.
11. Premature Abstraction
Problem: Creating abstractions before you understand the problem leads to wrong abstractions that are hard to change.
interface DataFetcher<T> {
fetch(): Promise<T>;
cache(): void;
invalidate(): void;
}
class GenericRepository<T> implements DataFetcher<T> {
constructor(private adapter: StorageAdapter<T>) {}
}
const userRepo = new GenericRepository(new UserAdapter());
async function fetchUsers(): Promise<User[]> {
return await db.query('SELECT * FROM users');
}
async function fetchUsersWithCache(): Promise<User[]> {
const cached = cache.get('users');
if (cached) return cached;
const users = await db.query('SELECT * FROM users');
cache.set('users', users);
return users;
}
Rule: "Duplication is far cheaper than the wrong abstraction." - Sandi Metz
12. Over-Engineering
Problem: Building for hypothetical future requirements adds complexity without value.
interface FizzBuzzStrategy {
applies(n: number): boolean;
execute(n: number): string;
}
class FizzStrategy implements FizzBuzzStrategy {
applies(n: number) { return n % 3 === 0; }
execute() { return 'Fizz'; }
}
class BuzzStrategy implements FizzBuzzStrategy {
applies(n: number) { return n % 5 === 0; }
execute() { return 'Buzz'; }
}
class FizzBuzzProcessor {
constructor(private strategies: FizzBuzzStrategy[]) {}
process(n: number): string {
return this.strategies
.filter(s => s.applies(n))
.map(s => s.execute(n))
.join('') || String(n);
}
}
const processor = new FizzBuzzProcessor([
new FizzStrategy(),
new BuzzStrategy(),
]);
function fizzBuzz(n: number): string {
if (n % 15 === 0) return 'FizzBuzz';
if (n % 3 === 0) return 'Fizz';
if (n % 5 === 0) return 'Buzz';
return String(n);
}
Rule: Solve today's problem. Refactor when requirements actually change.
13. Copy-Paste Programming
Problem: Duplicated code means duplicated bugs and maintenance burden.
function createUser(data: UserInput) {
if (!data.email) throw new Error('Email required');
if (!data.email.includes('@')) throw new Error('Invalid email');
if (data.email.length > 255) throw new Error('Email too long');
}
function updateUser(id: string, data: UserInput) {
if (!data.email) throw new Error('Email required');
if (!data.email.includes('@')) throw new Error('Invalid email');
if (data.email.length > 255) throw new Error('Email too long');
}
function inviteUser(data: UserInput) {
if (!data.email) throw new Error('Email required');
if (!data.email.includes('@')) throw new Error('Invalid email');
if (data.email.length > 255) throw new Error('Email too long');
}
function validateEmail(email: string): void {
if (!email) throw new Error('Email required');
if (!email.includes('@')) throw new Error('Invalid email');
if (email.length > 255) throw new Error('Email too long');
}
function createUser(data: UserInput) {
validateEmail(data.email);
}
function updateUser(id: string, data: UserInput) {
validateEmail(data.email);
}
function inviteUser(data: UserInput) {
validateEmail(data.email);
}
Rule: If you copy-paste, you're probably missing an abstraction.
14. God Objects
Problem: Classes that know too much and do too much become unmaintainable.
class ApplicationManager {
users: User[] = [];
orders: Order[] = [];
products: Product[] = [];
createUser() { }
deleteUser() { }
authenticateUser() { }
createOrder() { }
cancelOrder() { }
refundOrder() { }
addProduct() { }
updateInventory() { }
generateSalesReport() { }
generateUserReport() { }
sendEmail() { }
sendSMS() { }
}
class UserService {
createUser() { }
deleteUser() { }
}
class AuthService {
authenticate() { }
}
class OrderService {
createOrder() { }
cancelOrder() { }
}
class NotificationService {
sendEmail() { }
sendSMS() { }
}
Rule: Each class should have one reason to change.
Comment Anti-Patterns
15. Commented-Out Code
Problem: Commented code is dead code. It confuses readers and never gets cleaned up.
function calculateTotal(items: Item[]) {
let total = 0;
for (const item of items) {
total += item.price;
}
return total;
}
function calculateTotal(items: Item[]) {
return items.reduce((total, item) => total + item.price, 0);
}
Rule: Delete commented code. Use version control for history.
16. Obvious Comments
Problem: Comments that restate the code add noise without value.
counter++;
if (user === null) {
return;
}
for (const item of items) {
total += item.price;
}
counter++;
if (!user) return;
const total = items.reduce((sum, item) => sum + item.price, 0);
if (user.isPremium && hoursUntilLaunch < 48) {
showEarlyAccess();
}
Rule: Don't comment WHAT, comment WHY (if not obvious).
17. TODO Sprawl
Problem: TODOs accumulate and never get done. They become invisible noise.
function processPayment(amount: number) {
return charge(amount);
}
function processPayment(amount: number) {
return charge(amount);
}
async function processPayment(amount: number) {
return await retry(() => charge(amount), { attempts: 3 });
}
Rule: TODOs belong in your issue tracker, not your code.
18. Outdated Comments
Problem: Comments that contradict the code are actively harmful.
function getUserName(user: User): string {
return user.email;
}
function processUser(user: User) {
database.save(user);
}
function oldFunction() {
}
function getUserEmail(user: User): string {
return user.email;
}
function saveUser(user: User) {
database.save(user);
}
function oldFunction() {
console.warn('oldFunction is deprecated');
return newFunction();
}
Rule: When you change code, update or delete related comments.
Control Flow Anti-Patterns
19. Exception-Driven Control Flow
Problem: Using exceptions for normal control flow is slow and hard to follow.
function findUser(id: string): User {
try {
return database.getUser(id);
} catch {
try {
return cache.getUser(id);
} catch {
try {
return createDefaultUser(id);
} catch {
throw new Error('Cannot get user');
}
}
}
}
function findUser(id: string): User | null {
const dbUser = database.getUser(id);
if (dbUser) return dbUser;
const cachedUser = cache.getUser(id);
if (cachedUser) return cachedUser;
return createDefaultUser(id);
}
Rule: Exceptions are for exceptional situations, not control flow.
20. Stringly-Typed Code
Problem: Using strings where enums or types would be safer leads to typos and missing cases.
function handleStatus(status: string) {
if (status === 'pending') { }
else if (status === 'Pending') { }
else if (status === 'active') { }
else if (status === 'actve') { }
}
user.role = 'admni';
type Status = 'pending' | 'active' | 'completed' | 'cancelled';
type UserRole = 'admin' | 'user' | 'guest';
function handleStatus(status: Status) {
switch (status) {
case 'pending': break;
case 'active': break;
case 'completed': break;
case 'cancelled': break;
}
}
user.role = 'admni';
Rule: Use types instead of strings for fixed sets of values.
21. Callback Hell
Problem: Nested callbacks create unreadable, hard-to-debug code.
getUser(userId, (err, user) => {
if (err) return handleError(err);
getOrders(user.id, (err, orders) => {
if (err) return handleError(err);
getOrderDetails(orders[0].id, (err, details) => {
if (err) return handleError(err);
processDetails(details, (err, result) => {
if (err) return handleError(err);
sendNotification(result, (err) => {
if (err) return handleError(err);
console.log('Done!');
});
});
});
});
});
async function processUserOrder(userId: string) {
try {
const user = await getUser(userId);
const orders = await getOrders(user.id);
const details = await getOrderDetails(orders[0].id);
const result = await processDetails(details);
await sendNotification(result);
console.log('Done!');
} catch (error) {
handleError(error);
}
}
Rule: Use async/await for asynchronous code.
Quick Reference
| Anti-Pattern | Fix |
|---|
| Cryptic abbreviations | Spell it out |
| Generic names | Reveal intent |
| Misleading names | Match name to behavior |
| Hungarian notation | Let types handle types |
| God functions | Extract smaller functions |
| Too many parameters | Use options object |
| Boolean flags | Named options or separate functions |
| Side effects in getters | Make mutations explicit |
| Different return types | Use discriminated unions |
| Deep nesting | Guard clauses |
| Premature abstraction | Wait for patterns to emerge |
| Over-engineering | Solve today's problem |
| Copy-paste | Extract shared logic |
| God objects | Single responsibility |
| Commented-out code | Delete it |
| Obvious comments | Let code self-document |
| TODO sprawl | Use issue tracker |
| Outdated comments | Update or delete |
| Exception control flow | Explicit conditionals |
| Stringly-typed | Use enums/unions |
| Callback hell | async/await |
Code Smells Catalog
Code smells are symptoms that indicate deeper problems. They're not bugs—the code works—but they signal design issues that make code harder to understand, change, and maintain.
Based on Martin Fowler's refactoring catalog with TypeScript examples.
Bloaters
Code that has grown too large to work with effectively.
Long Method
Symptoms:
- Function exceeds 20 lines
- Multiple levels of abstraction in one function
- Comments separating "sections" within a function
- Difficult to name—does too many things
async function processCheckout(cart: Cart, user: User) {
if (!cart.items.length) throw new Error('Empty cart');
for (const item of cart.items) {
const product = await getProduct(item.productId);
if (product.stock < item.quantity) {
throw new Error(`Insufficient stock for ${product.name}`);
}
}
let subtotal = 0;
for (const item of cart.items) {
const product = await getProduct(item.productId);
subtotal += product.price * item.quantity;
}
const tax = subtotal * 0.1;
const shipping = subtotal > 100 ? 0 : 10;
const total = subtotal + tax + shipping;
const paymentIntent = await stripe.paymentIntents.create({
amount: Math.round(total * 100),
currency: 'usd',
});
const order = await db.orders.create({
userId: user.id,
items: cart.items,
total,
paymentIntentId: paymentIntent.id,
});
await sendEmail(user.email, 'Order Confirmed', `Order #${order.id}`);
for (const item of cart.items) {
await db.products.update(item.productId, {
stock: { decrement: item.quantity },
});
}
return order;
}
Refactoring: Extract Method
async function processCheckout(cart: Cart, user: User) {
await validateCart(cart);
const totals = await calculateTotals(cart);
const payment = await processPayment(totals.total);
const order = await createOrder(user, cart, totals, payment);
await sendOrderConfirmation(user, order);
await updateInventory(cart.items);
return order;
}
async function validateCart(cart: Cart): Promise<void> {
if (!cart.items.length) throw new Error('Empty cart');
for (const item of cart.items) {
const product = await getProduct(item.productId);
if (product.stock < item.quantity) {
throw new Error(`Insufficient stock for ${product.name}`);
}
}
}
async function calculateTotals(cart: Cart): Promise<OrderTotals> {
const subtotal = await calculateSubtotal(cart.items);
const tax = subtotal * 0.1;
const shipping = subtotal > 100 ? 0 : 10;
return { subtotal, tax, shipping, total: subtotal + tax + shipping };
}
Prevention: If you're adding a comment to separate sections, extract a function instead.
Large Class
Symptoms:
- Class has 10+ methods
- Class has 10+ fields
- Class name includes "Manager", "Processor", "Handler" doing everything
- You can't summarize what the class does in one sentence
class UserManager {
createUser(data: UserInput) { }
updateUser(id: string, data: UserInput) { }
deleteUser(id: string) { }
getUser(id: string) { }
listUsers(filters: UserFilters) { }
login(email: string, password: string) { }
logout(userId: string) { }
resetPassword(email: string) { }
verifyEmail(token: string) { }
checkPermission(userId: string, resource: string) { }
assignRole(userId: string, role: string) { }
updateProfile(userId: string, profile: Profile) { }
uploadAvatar(userId: string, file: File) { }
sendWelcomeEmail(userId: string) { }
sendPasswordResetEmail(email: string) { }
trackLogin(userId: string) { }
getLoginHistory(userId: string) { }
}
Refactoring: Extract Class
class UserRepository {
create(data: UserInput) { }
update(id: string, data: UserInput) { }
delete(id: string) { }
findById(id: string) { }
findMany(filters: UserFilters) { }
}
class AuthService {
login(email: string, password: string) { }
logout(userId: string) { }
resetPassword(email: string) { }
verifyEmail(token: string) { }
}
class AuthorizationService {
checkPermission(userId: string, resource: string) { }
assignRole(userId: string, role: string) { }
}
class ProfileService {
updateProfile(userId: string, profile: Profile) { }
uploadAvatar(userId: string, file: File) { }
}
class UserNotificationService {
sendWelcomeEmail(userId: string) { }
sendPasswordResetEmail(email: string) { }
}
Prevention: Each class should have one reason to change.
Long Parameter List
Symptoms:
- Function has 4+ parameters
- Parameters are often passed together
- Hard to remember parameter order
- Boolean parameters with unclear meaning
function createReport(
title: string,
startDate: Date,
endDate: Date,
format: string,
includeCharts: boolean,
includeTables: boolean,
groupBy: string,
sortBy: string,
sortOrder: string,
limit: number,
userId: string
) {
}
Refactoring: Introduce Parameter Object
interface ReportOptions {
title: string;
dateRange: {
start: Date;
end: Date;
};
format: 'pdf' | 'excel' | 'csv';
includes?: {
charts?: boolean;
tables?: boolean;
};
sorting?: {
field: string;
order: 'asc' | 'desc';
};
groupBy?: string;
limit?: number;
userId: string;
}
function createReport(options: ReportOptions) {
const { title, dateRange, format, includes, sorting, limit = 100 } = options;
}
Prevention: If parameters travel together, they belong together.
Data Clumps
Symptoms:
- Same 3+ fields appear together in multiple places
- Functions pass the same group of parameters
- Classes have fields that are always used together
function calculateDistance(
startLat: number, startLng: number,
endLat: number, endLng: number
) { }
function formatAddress(
street: string, city: string, state: string, zip: string
) { }
class Delivery {
pickupStreet: string;
pickupCity: string;
pickupState: string;
pickupZip: string;
dropoffStreet: string;
dropoffCity: string;
dropoffState: string;
dropoffZip: string;
}
Refactoring: Extract Class
interface Coordinates {
latitude: number;
longitude: number;
}
interface Address {
street: string;
city: string;
state: string;
zip: string;
}
function calculateDistance(start: Coordinates, end: Coordinates) { }
function formatAddress(address: Address) { }
class Delivery {
pickup: Address;
dropoff: Address;
}
Prevention: When you see fields traveling together, introduce a class.
Primitive Obsession
Symptoms:
- Using strings/numbers where a class would be clearer
- Validation logic repeated for the same concept
- Magic strings or numbers scattered in code
function processOrder(
userId: string,
email: string,
phone: string,
amount: number,
currency: string,
status: string
) {
if (!email.includes('@')) throw new Error('Invalid email');
if (currency === 'USD') { }
else if (currency === 'EUR') { }
if (status === 'pending') { }
}
Refactoring: Replace Primitive with Object
class Email {
private constructor(private readonly value: string) {}
static create(value: string): Email {
if (!value.includes('@')) throw new Error('Invalid email');
return new Email(value.toLowerCase());
}
toString() { return this.value; }
}
class Money {
constructor(
private readonly amount: number,
private readonly currency: Currency
) {}
add(other: Money): Money {
if (this.currency !== other.currency) {
throw new Error('Currency mismatch');
}
return new Money(this.amount + other.amount, this.currency);
}
}
type OrderStatus = 'pending' | 'confirmed' | 'shipped' | 'delivered';
function processOrder(
userId: UserId,
email: Email,
amount: Money,
status: OrderStatus
) {
}
Prevention: If you validate a primitive the same way multiple times, wrap it.
Object-Orientation Abusers
Incorrect or incomplete application of OO principles.
Switch Statements
Symptoms:
- Same switch/if-else chain in multiple places
- Adding a new case requires changes in multiple files
- Switches based on type codes
function calculatePay(employee: Employee): number {
switch (employee.type) {
case 'hourly':
return employee.hoursWorked * employee.hourlyRate;
case 'salaried':
return employee.annualSalary / 12;
case 'commissioned':
return employee.baseSalary + employee.sales * employee.commissionRate;
default:
throw new Error('Unknown employee type');
}
}
function getTimeOffDays(employee: Employee): number {
switch (employee.type) {
case 'hourly': return 10;
case 'salaried': return 20;
case 'commissioned': return 15;
default: return 0;
}
}
Refactoring: Replace Conditional with Polymorphism
interface Employee {
calculatePay(): number;
getTimeOffDays(): number;
}
class HourlyEmployee implements Employee {
constructor(
private hoursWorked: number,
private hourlyRate: number
) {}
calculatePay() { return this.hoursWorked * this.hourlyRate; }
getTimeOffDays() { return 10; }
}
class SalariedEmployee implements Employee {
constructor(private annualSalary: number) {}
calculatePay() { return this.annualSalary / 12; }
getTimeOffDays() { return 20; }
}
class CommissionedEmployee implements Employee {
constructor(
private baseSalary: number,
private sales: number,
private commissionRate: number
) {}
calculatePay() {
return this.baseSalary + this.sales * this.commissionRate;
}
getTimeOffDays() { return 15; }
}
Prevention: If switches on type appear in multiple places, use polymorphism.
Temporary Field
Symptoms:
- Fields that are only set in certain situations
- Null checks scattered throughout the class
- Fields used by only some methods
class Order {
items: OrderItem[];
customer: Customer;
discountPercentage?: number;
discountReason?: string;
discountApprover?: string;
trackingNumber?: string;
carrier?: string;
estimatedDelivery?: Date;
calculateTotal() {
let total = this.items.reduce((sum, i) => sum + i.price, 0);
if (this.discountPercentage) {
total *= (1 - this.discountPercentage / 100);
}
return total;
}
}
Refactoring: Extract Class or Introduce Null Object
class Order {
items: OrderItem[];
customer: Customer;
discount: Discount = Discount.none();
shipping?: ShippingInfo;
calculateTotal() {
const subtotal = this.items.reduce((sum, i) => sum + i.price, 0);
return this.discount.applyTo(subtotal);
}
}
class Discount {
private constructor(
private percentage: number,
private reason: string,
private approver: string | null
) {}
static none() { return new Discount(0, '', null); }
static create(percentage: number, reason: string, approver: string) {
return new Discount(percentage, reason, approver);
}
applyTo(amount: number) {
return amount * (1 - this.percentage / 100);
}
}
interface ShippingInfo {
trackingNumber: string;
carrier: string;
estimatedDelivery: Date;
}
Prevention: If fields are only used together, extract them to a class.
Change Preventers
Code structures that make changes difficult.
Divergent Change
Symptoms:
- One class is modified for multiple unrelated reasons
- Changes in different domains affect the same file
- "I need to change X, Y, and Z in this file"
class ReportGenerator {
generatePDF(data: ReportData) { }
generateExcel(data: ReportData) { }
generateCSV(data: ReportData) { }
fetchFromDatabase(query: string) { }
fetchFromAPI(endpoint: string) { }
calculateTotals(data: ReportData) { }
applyFilters(data: ReportData) { }
}
Refactoring: Extract Class
class ReportFormatter {
toPDF(data: ReportData) { }
toExcel(data: ReportData) { }
toCSV(data: ReportData) { }
}
class DataFetcher {
fromDatabase(query: string) { }
fromAPI(endpoint: string) { }
}
class ReportCalculator {
calculateTotals(data: ReportData) { }
applyFilters(data: ReportData) { }
}
class ReportGenerator {
constructor(
private fetcher: DataFetcher,
private calculator: ReportCalculator,
private formatter: ReportFormatter
) {}
generate(source: DataSource, format: Format): Report {
const data = this.fetcher.fetch(source);
const processed = this.calculator.process(data);
return this.formatter.format(processed, format);
}
}
Prevention: Each class should have one reason to change.
Shotgun Surgery
Symptoms:
- A single change requires editing many files
- Related code is scattered across the codebase
- Easy to miss one of the required changes
interface User { name: string; email: string; }
app.post('/users', (req) => {
const { name, email } = req.body;
});
function validateUser(data: unknown) {
if (!data.name) throw new Error('Name required');
if (!data.email) throw new Error('Email required');
}
function createUser(name: string, email: string) {
db.query('INSERT INTO users (name, email) VALUES (?, ?)', [name, email]);
}
Refactoring: Move Method, Inline Class
interface User {
name: string;
email: string;
phone?: string;
}
const UserSchema = z.object({
name: z.string().min(1),
email: z.string().email(),
phone: z.string().optional(),
});
class UserRepository {
create(data: User) {
const validated = UserSchema.parse(data);
return db.users.create(validated);
}
}
app.post('/users', async (req) => {
const user = await userRepository.create(req.body);
return user;
});
Prevention: Keep related behavior together.
Feature Envy
Symptoms:
- A method uses more features of another class than its own
- Lots of getter calls on other objects
- Logic that should belong to the data it operates on
class OrderPrinter {
print(order: Order) {
console.log(`Order: ${order.getId()}`);
console.log(`Customer: ${order.getCustomer().getName()}`);
console.log(`Address: ${order.getCustomer().getAddress().getFullAddress()}`);
let total = 0;
for (const item of order.getItems()) {
const price = item.getProduct().getPrice();
const qty = item.getQuantity();
const discount = item.getProduct().getDiscount();
const lineTotal = price * qty * (1 - discount);
total += lineTotal;
console.log(`${item.getProduct().getName()}: $${lineTotal}`);
}
console.log(`Total: $${total}`);
}
}
Refactoring: Move Method
class Order {
getFormattedSummary(): string {
return [
`Order: ${this.id}`,
`Customer: ${this.customer.name}`,
`Address: ${this.customer.address.format()}`,
...this.items.map(item => item.format()),
`Total: $${this.calculateTotal()}`,
].join('\n');
}
calculateTotal(): number {
return this.items.reduce((sum, item) => sum + item.calculateLineTotal(), 0);
}
}
class OrderItem {
calculateLineTotal(): number {
return this.product.price * this.quantity * (1 - this.product.discount);
}
format(): string {
return `${this.product.name}: $${this.calculateLineTotal()}`;
}
}
class OrderPrinter {
print(order: Order) {
console.log(order.getFormattedSummary());
}
}
Prevention: Put behavior with the data it needs.
Dispensables
Code that serves no purpose and should be removed.
Dead Code
Symptoms:
- Unreachable code after return/throw
- Unused variables, parameters, or functions
- Commented-out code
- Obsolete feature flags
function processPayment(amount: number) {
if (amount <= 0) {
throw new Error('Invalid amount');
console.log('This never runs');
}
const result = charge(amount);
return result;
sendReceipt(result);
updateAnalytics(result);
}
function legacyPaymentProcessor() {
}
const FEATURE_FLAG_OLD = false;
Refactoring: Remove Dead Code
function processPayment(amount: number) {
if (amount <= 0) {
throw new Error('Invalid amount');
}
const result = charge(amount);
sendReceipt(result);
updateAnalytics(result);
return result;
}
Prevention: Use your IDE's "find unused" feature regularly.
Speculative Generality
Symptoms:
- Abstract classes with only one implementation
- Parameters or methods that are never used
- "We might need this someday" code
- Complex framework for simple problem
interface PaymentProcessor {
process(payment: Payment): Promise<Result>;
refund(paymentId: string): Promise<Result>;
void(paymentId: string): Promise<Result>;
partialRefund(paymentId: string, amount: number): Promise<Result>;
recurring(subscription: Subscription): Promise<Result>;
}
class StripeProcessor implements PaymentProcessor {
process(payment: Payment) { }
refund(paymentId: string) { }
void() { throw new Error('Not implemented'); }
partialRefund() { throw new Error('Not implemented'); }
recurring() { throw new Error('Not implemented'); }
}
Refactoring: Collapse Hierarchy, Remove Parameter
class PaymentService {
constructor(private stripe: Stripe) {}
async charge(amount: number, customer: string) {
return this.stripe.charges.create({ amount, customer });
}
async refund(chargeId: string) {
return this.stripe.refunds.create({ charge: chargeId });
}
}
Prevention: YAGNI—You Aren't Gonna Need It.
Duplicate Code
Symptoms:
- Same code structure in multiple places
- Copy-pasted code with minor variations
- Parallel inheritance hierarchies
class AdminController {
async getUsers(req: Request) {
const page = parseInt(req.query.page as string) || 1;
const limit = parseInt(req.query.limit as string) || 10;
const offset = (page - 1) * limit;
const users = await db.users.findMany({ skip: offset, take: limit });
const total = await db.users.count();
return {
data: users,
meta: { page, limit, total, pages: Math.ceil(total / limit) }
};
}
}
class ProductController {
async getProducts(req: Request) {
const page = parseInt(req.query.page as string) || 1;
const limit = parseInt(req.query.limit as string) || 10;
const offset = (page - 1) * limit;
const products = await db.products.findMany({ skip: offset, take: limit });
const total = await db.products.count();
return {
data: products,
meta: { page, limit, total, pages: Math.ceil(total / limit) }
};
}
}
Refactoring: Extract Function, Extract Class
interface PaginationParams {
page: number;
limit: number;
}
function parsePagination(query: Record<string, string>): PaginationParams {
return {
page: parseInt(query.page) || 1,
limit: parseInt(query.limit) || 10,
};
}
async function paginate<T>(
query: { findMany: Function; count: Function },
params: PaginationParams
) {
const { page, limit } = params;
const offset = (page - 1) * limit;
const [data, total] = await Promise.all([
query.findMany({ skip: offset, take: limit }),
query.count(),
]);
return {
data,
meta: { page, limit, total, pages: Math.ceil(total / limit) }
};
}
class AdminController {
async getUsers(req: Request) {
return paginate(db.users, parsePagination(req.query));
}
}
class ProductController {
async getProducts(req: Request) {
return paginate(db.products, parsePagination(req.query));
}
}
Prevention: If you copy-paste, extract immediately.
Couplers
Code with excessive coupling between classes.
Message Chains
Symptoms:
- Long chains of method calls:
a.getB().getC().getD().doSomething()
- Client depends on navigation structure
- Changes to intermediate objects break clients
function getManagerEmail(employee: Employee): string {
return employee
.getDepartment()
.getManager()
.getContactInfo()
.getEmail()
.toLowerCase();
}
function sendReport(order: Order) {
const warehouse = order
.getShipment()
.getRoute()
.getDestination()
.getWarehouse();
warehouse.receive(order);
}
Refactoring: Hide Delegate
class Employee {
getManagerEmail(): string {
return this.department.getManagerEmail();
}
}
class Department {
getManagerEmail(): string {
return this.manager.email.toLowerCase();
}
}
function getManagerEmail(employee: Employee): string {
return employee.getManagerEmail();
}
class Order {
getDestinationWarehouse(): Warehouse {
return this.shipment.getDestinationWarehouse();
}
}
Prevention: Follow the Law of Demeter—only talk to immediate friends.
Middle Man
Symptoms:
- Class mostly delegates to another class
- No added value, just passing through
- Interface mirrors another class
class PersonWrapper {
private person: Person;
getName() { return this.person.getName(); }
setName(name: string) { this.person.setName(name); }
getAge() { return this.person.getAge(); }
setAge(age: number) { this.person.setAge(age); }
getAddress() { return this.person.getAddress(); }
setAddress(addr: Address) { this.person.setAddress(addr); }
}
Refactoring: Remove Middle Man
class PersonView {
constructor(private person: Person) {}
getDisplayName(): string {
return `${this.person.getName()} (${this.person.getAge()})`;
}
}
Prevention: Only add indirection when it provides value.
Quick Reference
| Category | Smell | Fix |
|---|
| Bloaters | Long Method | Extract Method |
| Large Class | Extract Class |
| Long Parameter List | Parameter Object |
| Data Clumps | Extract Class |
| Primitive Obsession | Replace with Object |
| OO Abusers | Switch Statements | Polymorphism |
| Temporary Field | Extract Class |
| Change Preventers | Divergent Change | Extract Class |
| Shotgun Surgery | Move Method |
| Feature Envy | Move Method |
| Dispensables | Dead Code | Delete it |
| Speculative Generality | Delete it |
| Duplicate Code | Extract |
| Couplers | Message Chains | Hide Delegate |
| Middle Man | Remove it |
Refactoring Catalog
Essential refactoring patterns with before/after examples. Each refactoring is a small, reversible transformation that improves code structure without changing behavior.
Based on Martin Fowler's refactoring catalog with TypeScript examples.
Composing Methods
Break down large methods into smaller, focused pieces.
Extract Function
Motivation: A code fragment that can be grouped together and named. The most common refactoring.
When to use:
- Code block has a comment explaining what it does
- Same code appears in multiple places
- Function is too long (20+ lines)
- Logic is at a different level of abstraction
function printInvoice(invoice: Invoice) {
console.log('=================');
console.log('=== INVOICE =====');
console.log('=================');
console.log(`Customer: ${invoice.customer}`);
console.log(`Date: ${invoice.date}`);
let total = 0;
for (const item of invoice.items) {
total += item.price * item.quantity;
}
console.log(`Total: $${total}`);
console.log('=================');
}
function printInvoice(invoice: Invoice) {
printHeader();
printDetails(invoice);
printTotal(invoice);
printFooter();
}
function printHeader() {
console.log('=================');
console.log('=== INVOICE =====');
console.log('=================');
}
function printDetails(invoice: Invoice) {
console.log(`Customer: ${invoice.customer}`);
console.log(`Date: ${invoice.date}`);
}
function printTotal(invoice: Invoice) {
const total = calculateTotal(invoice.items);
console.log(`Total: $${total}`);
}
function calculateTotal(items: InvoiceItem[]): number {
return items.reduce((sum, item) => sum + item.price * item.quantity, 0);
}
function printFooter() {
console.log('=================');
}
Mechanics:
- Create new function named after what it does (not how)
- Copy the code fragment to the new function
- Pass any needed variables as parameters
- Replace the original code with a call to the new function
Inline Function
Motivation: The opposite of Extract Function. The function body is as clear as the name, or the function is only delegating.
When to use:
- Function body is as obvious as its name
- You have a group of badly factored functions and want to re-extract differently
- Too much indirection
function moreThanFiveOrders(customer: Customer): boolean {
return customer.orders.length > 5;
}
function getDiscount(customer: Customer): number {
if (moreThanFiveOrders(customer)) {
return 0.1;
}
return 0;
}
function getDiscount(customer: Customer): number {
if (customer.orders.length > 5) {
return 0.1;
}
return 0;
}
Mechanics:
- Check function isn't polymorphic (overridden in subclasses)
- Find all callers
- Replace each call with the function body
- Delete the function
Replace Temp with Query
Motivation: Temporary variables can be a problem—they make functions longer and block Extract Function.
When to use:
- A temp is assigned once and used multiple times
- The calculation could be a method
- You want to extract part of a function
function getPrice(order: Order): number {
const basePrice = order.quantity * order.itemPrice;
const discount = Math.max(0, order.quantity - 100) * order.itemPrice * 0.05;
const shipping = Math.min(basePrice * 0.1, 50);
return basePrice - discount + shipping;
}
function getPrice(order: Order): number {
return basePrice(order) - discount(order) + shipping(order);
}
function basePrice(order: Order): number {
return order.quantity * order.itemPrice;
}
function discount(order: Order): number {
return Math.max(0, order.quantity - 100) * order.itemPrice * 0.05;
}
function shipping(order: Order): number {
return Math.min(basePrice(order) * 0.1, 50);
}
Mechanics:
- Check the temp is only assigned once
- Extract the assignment into a function
- Replace references to the temp with function calls
- Remove the temp declaration
Introduce Explaining Variable
Motivation: Complex expressions are hard to read. Break them into named pieces.
When to use:
- A complex expression that's hard to understand
- Multiple conditions in an if statement
- Mathematical formulas
function calculatePrice(order: Order): number {
return order.quantity * order.itemPrice -
Math.max(0, order.quantity - 100) * order.itemPrice * 0.05 +
Math.min(order.quantity * order.itemPrice * 0.1, 50);
}
function isEligibleForDiscount(user: User): boolean {
return user.age >= 65 ||
(user.memberSince < new Date('2020-01-01') && user.purchaseCount > 10) ||
user.isEmployee;
}
function calculatePrice(order: Order): number {
const basePrice = order.quantity * order.itemPrice;
const quantityDiscount = Math.max(0, order.quantity - 100) * order.itemPrice * 0.05;
const shipping = Math.min(basePrice * 0.1, 50);
return basePrice - quantityDiscount + shipping;
}
function isEligibleForDiscount(user: User): boolean {
const isSenior = user.age >= 65;
const isLoyalCustomer = user.memberSince < new Date('2020-01-01') &&
user.purchaseCount > 10;
const isEmployee = user.isEmployee;
return isSenior || isLoyalCustomer || isEmployee;
}
Mechanics:
- Identify a complex expression or sub-expression
- Create a variable with a meaningful name
- Assign the expression to the variable
- Replace the expression with the variable
Moving Features
Move code to where it belongs.
Move Function
Motivation: Functions should live with the data they use most.
When to use:
- Function uses more features of another class
- Function is in the wrong module
- Coupling would be reduced by moving
class InterestCalculator {
calculateInterest(account: Account, days: number): number {
const balance = account.getBalance();
const rate = account.getInterestRate();
const type = account.getType();
if (type === 'savings') {
return balance * rate * days / 365;
} else if (type === 'checking') {
return balance * (rate - 0.01) * days / 365;
}
return 0;
}
}
class Account {
private balance: number;
private interestRate: number;
private type: 'savings' | 'checking';
calculateInterest(days: number): number {
if (this.type === 'savings') {
return this.balance * this.interestRate * days / 365;
} else if (this.type === 'checking') {
return this.balance * (this.interestRate - 0.01) * days / 365;
}
return 0;
}
}
Mechanics:
- Look at what the function references—does it use more from elsewhere?
- Check if it should be a method on one of its arguments
- Move the function
- Update all callers
Extract Class
Motivation: A class is doing too much. Split it based on responsibilities.
When to use:
- Class has too many fields
- Class has too many methods
- Subsets of data/methods are used together
- Class name includes "And" or "Manager"
class Person {
name: string;
officeAreaCode: string;
officeNumber: string;
homeAreaCode: string;
homeNumber: string;
getOfficePhone(): string {
return `(${this.officeAreaCode}) ${this.officeNumber}`;
}
getHomePhone(): string {
return `(${this.homeAreaCode}) ${this.homeNumber}`;
}
}
class PhoneNumber {
constructor(
private areaCode: string,
private number: string
) {}
format(): string {
return `(${this.areaCode}) ${this.number}`;
}
}
class Person {
name: string;
officePhone: PhoneNumber;
homePhone: PhoneNumber;
getOfficePhone(): string {
return this.officePhone.format();
}
getHomePhone(): string {
return this.homePhone.format();
}
}
Mechanics:
- Identify a subset of data and methods that belong together
- Create a new class
- Move fields and methods to the new class
- Create a link from old class to new class
Hide Delegate
Motivation: Reduce coupling by hiding the object structure from clients.
When to use:
- Clients navigate through one object to get to another
- Changes to the delegate affect all clients
- You're exposing internal structure
class Person {
department: Department;
}
class Department {
manager: Person;
}
const manager = person.department.manager;
class Person {
private department: Department;
getManager(): Person {
return this.department.manager;
}
}
const manager = person.getManager();
Mechanics:
- Create a delegating method on the server
- Replace client calls to the delegate with calls to the server
- Consider making the delegate field private
Organizing Data
Improve how data is structured and accessed.
Replace Magic Number with Constant
Motivation: Magic numbers obscure meaning and are easy to mistype.
When to use:
- Numbers appear in code without explanation
- Same number appears in multiple places
- The number has domain meaning
function potentialEnergy(mass: number, height: number): number {
return mass * height * 9.81;
}
function calculateDiscount(total: number): number {
if (total > 100) {
return total * 0.1;
}
return 0;
}
const GRAVITATIONAL_CONSTANT = 9.81;
const DISCOUNT_THRESHOLD = 100;
const DISCOUNT_RATE = 0.1;
function potentialEnergy(mass: number, height: number): number {
return mass * height * GRAVITATIONAL_CONSTANT;
}
function calculateDiscount(total: number): number {
if (total > DISCOUNT_THRESHOLD) {
return total * DISCOUNT_RATE;
}
return 0;
}
Mechanics:
- Create a constant with a meaningful name
- Replace the magic number with the constant
- Search for other occurrences of the same number
Replace Primitive with Object
Motivation: Primitives with behavior should be objects.
When to use:
- Same validation logic for a primitive appears multiple times
- Formatting/parsing logic is scattered
- The value has business rules
function validateOrder(order: Order) {
const phone = order.phone;
if (!phone.match(/^\d{10}$/)) {
throw new Error('Invalid phone');
}
}
function formatPhone(phone: string): string {
return `(${phone.slice(0,3)}) ${phone.slice(3,6)}-${phone.slice(6)}`;
}
class PhoneNumber {
private readonly value: string;
constructor(phone: string) {
if (!phone.match(/^\d{10}$/)) {
throw new Error('Invalid phone number');
}
this.value = phone;
}
format(): string {
return `(${this.value.slice(0,3)}) ${this.value.slice(3,6)}-${this.value.slice(6)}`;
}
getAreaCode(): string {
return this.value.slice(0, 3);
}
toString(): string {
return this.value;
}
}
const phone = new PhoneNumber('5551234567');
console.log(phone.format());
Mechanics:
- Create a class for the value
- Add validation in constructor
- Add any formatting/parsing methods
- Replace usages of the primitive
Encapsulate Collection
Motivation: Exposing a collection allows clients to modify it without the owner knowing.
When to use:
- A getter returns a raw collection
- Clients are adding/removing items directly
- Collection modifications should trigger other behavior
class Course {
name: string;
}
class Person {
courses: Course[] = [];
getCourses(): Course[] {
return this.courses;
}
}
person.getCourses().push(newCourse);
person.getCourses().length = 0;
class Person {
private courses: Course[] = [];
getCourses(): readonly Course[] {
return [...this.courses];
}
addCourse(course: Course): void {
this.courses.push(course);
}
removeCourse(course: Course): void {
const index = this.courses.indexOf(course);
if (index > -1) {
this.courses.splice(index, 1);
}
}
get numberOfCourses(): number {
return this.courses.length;
}
}
Mechanics:
- Add methods for modifying the collection
- Return a copy or readonly view from the getter
- Replace direct modifications with method calls
Simplifying Conditionals
Make conditional logic easier to understand.
Decompose Conditional
Motivation: Complex conditionals are hard to read. Extract into well-named functions.
When to use:
- Conditional has complex conditions
- Then/else branches have substantial code
- The logic isn't immediately clear
function calculateCharge(date: Date, quantity: number): number {
if (date.getMonth() >= 5 && date.getMonth() <= 8) {
return quantity * 1.2 + (quantity > 100 ? quantity * 0.05 : 0);
} else {
return quantity * 1.0 + (quantity > 50 ? quantity * 0.03 : 0);
}
}
function calculateCharge(date: Date, quantity: number): number {
if (isSummer(date)) {
return summerCharge(quantity);
} else {
return regularCharge(quantity);
}
}
function isSummer(date: Date): boolean {
return date.getMonth() >= 5 && date.getMonth() <= 8;
}
function summerCharge(quantity: number): number {
const baseCharge = quantity * 1.2;
const bulkDiscount = quantity > 100 ? quantity * 0.05 : 0;
return baseCharge + bulkDiscount;
}
function regularCharge(quantity: number): number {
const baseCharge = quantity * 1.0;
const bulkDiscount = quantity > 50 ? quantity * 0.03 : 0;
return baseCharge + bulkDiscount;
}
Mechanics:
- Extract the condition into a function
- Extract the then-branch into a function
- Extract the else-branch into a function
Consolidate Conditional Expression
Motivation: Multiple conditions with the same result should be combined.
When to use:
- Several conditions return the same value
- The conditions are really checking one thing
- Combining makes intent clearer
function disabilityAmount(employee: Employee): number {
if (employee.seniority < 2) return 0;
if (employee.monthsDisabled > 12) return 0;
if (employee.isPartTime) return 0;
return employee.salary * 0.6;
}
function disabilityAmount(employee: Employee): number {
if (isNotEligibleForDisability(employee)) return 0;
return employee.salary * 0.6;
}
function isNotEligibleForDisability(employee: Employee): boolean {
return employee.seniority < 2 ||
employee.monthsDisabled > 12 ||
employee.isPartTime;
}
Mechanics:
- Combine conditions using logical operators
- Extract the combined condition into a function
- Give it a meaningful name
Replace Nested Conditional with Guard Clauses
Motivation: Deeply nested conditionals are hard to follow. Use early returns.
When to use:
- Deep nesting (more than 2 levels)
- Some branches are exceptional/edge cases
- Happy path is buried in else clauses
function getPayAmount(employee: Employee): number {
let result: number;
if (employee.isSeparated) {
result = 0;
} else {
if (employee.isRetired) {
result = employee.pension;
} else {
if (employee.isOnLeave) {
result = employee.salary * 0.5;
} else {
result = employee.salary;
}
}
}
return result;
}
function getPayAmount(employee: Employee): number {
if (employee.isSeparated) return 0;
if (employee.isRetired) return employee.pension;
if (employee.isOnLeave) return employee.salary * 0.5;
return employee.salary;
}
Mechanics:
- Identify edge cases
- Replace each with a guard clause (early return)
- Remove unnecessary else clauses
Replace Conditional with Polymorphism
Motivation: Type-based conditionals often indicate missing polymorphism.
When to use:
- Switching on type code
- Same switch appears in multiple places
- Each case has substantially different behavior
type BirdType = 'european' | 'african' | 'norwegian_blue';
function getSpeed(bird: { type: BirdType; voltage?: number }): number {
switch (bird.type) {
case 'european':
return 35;
case 'african':
return 40 - 2 * bird.numberOfCoconuts;
case 'norwegian_blue':
return bird.voltage > 100 ? 20 : 0;
default:
return 0;
}
}
function getPlumage(bird: { type: BirdType }): string {
switch (bird.type) {
case 'european':
return 'average';
case 'african':
return bird.numberOfCoconuts > 2 ? 'tired' : 'average';
case 'norwegian_blue':
return bird.voltage > 100 ? 'scorched' : 'beautiful';
default:
return 'unknown';
}
}
interface Bird {
getSpeed(): number;
getPlumage(): string;
}
class EuropeanSwallow implements Bird {
getSpeed() { return 35; }
getPlumage() { return 'average'; }
}
class AfricanSwallow implements Bird {
constructor(private numberOfCoconuts: number) {}
getSpeed() { return 40 - 2 * this.numberOfCoconuts; }
getPlumage() { return this.numberOfCoconuts > 2 ? 'tired' : 'average'; }
}
class NorwegianBlueParrot implements Bird {
constructor(private voltage: number) {}
getSpeed() { return this.voltage > 100 ? 20 : 0; }
getPlumage() { return this.voltage > 100 ? 'scorched' : 'beautiful'; }
}
function createBird(data: BirdData): Bird {
switch (data.type) {
case 'european': return new EuropeanSwallow();
case 'african': return new AfricanSwallow(data.numberOfCoconuts);
case 'norwegian_blue': return new NorwegianBlueParrot(data.voltage);
}
}
Mechanics:
- Create interface/base class
- Create a class for each type
- Move switch logic into each class
- Replace switch with polymorphic call
Simplifying Function Calls
Make functions easier to call and understand.
Rename Function
Motivation: Good names are the best documentation. If you can't name it, you don't understand it.
When to use:
- Name doesn't describe what the function does
- Name is misleading
- Name is too technical for the domain
function calc(a: number, b: number): number { }
function process(data: unknown): void { }
function handle(event: Event): void { }
function inv(customer: Customer): Invoice { }
function calculateCompoundInterest(principal: number, rate: number): number { }
function validateAndSaveUserProfile(profile: UserProfile): void { }
function trackButtonClick(event: MouseEvent): void { }
function generateInvoiceForCustomer(customer: Customer): Invoice { }
Mechanics:
- Choose a better name (this is the hard part)
- Create new function with new name
- Copy body to new function
- Update all callers
- Remove old function
Introduce Parameter Object
Motivation: Groups of parameters that travel together should be a single object.
When to use:
- Same group of parameters appears in multiple functions
- Parameters have a clear relationship
- You want to add behavior to the group
function getTotalSales(startDate: Date, endDate: Date): number { }
function getAverageOrders(startDate: Date, endDate: Date): number { }
function generateReport(startDate: Date, endDate: Date, format: string): Report { }
class DateRange {
constructor(
readonly start: Date,
readonly end: Date
) {
if (start > end) throw new Error('Invalid date range');
}
contains(date: Date): boolean {
return date >= this.start && date <= this.end;
}
get durationInDays(): number {
const ms = this.end.getTime() - this.start.getTime();
return Math.ceil(ms / (1000 * 60 * 60 * 24));
}
}
function getTotalSales(dateRange: DateRange): number { }
function getAverageOrders(dateRange: DateRange): number { }
function generateReport(dateRange: DateRange, format: string): Report { }
Mechanics:
- Create a class for the parameter group
- Add validation in constructor
- Add any relevant methods
- Replace parameter lists with the new object
Replace Parameter with Method
Motivation: A parameter that can be derived from other available information is redundant.
When to use:
