- 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

# 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 };
+ }

# 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);
+ }

# 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) { /* ... */ }
+ }

# 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('[email protected]')
+ .password('secure123')
+ .name('Test User')
+ .address(address)
+ .build();

# 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);
+ }
+ }

# BAD: Using primitives for domain concepts
- function sendEmail(to, subject, body) { /* ... */ }
- sendEmail('[email protected]', '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('[email protected]');
+ const phone = new PhoneNumber('1', '555-1234');

# 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);

# 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 {
+ return Result.combine([
+ validateOrderExists(order),
+ validateUserExists(order),
+ validateUserActive(order.user),
+ validateOrderTotal(order)
+ ]).flatMap(() => processOrder(order));
+ }

# 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

# 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
+ }
+ }

# 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('');
+ }

# 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

## Design Patterns for Refactoring

### Strategy Pattern

### Chain of Responsibility

1. PREPARE
 - Ensure tests exist (write them if missing)
 - Commit current state
 - Create feature branch

2. IDENTIFY
 - Find the code smell to address
 - Understand what the code does
 - Plan the refactoring

3. REFACTOR (small steps)
 - Make one small change
 - Run tests
 - Commit if tests pass
 - Repeat

4. VERIFY
 - All tests pass
 - Manual testing if needed
 - Performance unchanged or improved

5. CLEAN UP
 - Update comments
 - Update documentation
 - Final commit