Problem: You Need to Change Code You're Afraid to Touch
Your codebase has a 500-line function with nested conditionals, global state mutations, and no tests. You need to add a feature or fix a bug, but every change breaks something unexpected.
You'll learn:
- How to add safety nets before refactoring
- The 4-step process to untangle complex code
- When to stop refactoring and ship
Time: 45 min | Level: Intermediate
Why This Happens
Spaghetti code emerges from deadline pressure, unclear requirements, and missing abstractions. Functions grow organically, coupling increases, and eventually nobody understands the full behavior.
Common symptoms:
- Fear of changing code because "it just works"
- Can't explain what a function does in one sentence
- Changes in one place break unrelated features
- No clear separation between business logic and I/O
Solution
Step 1: Add Characterization Tests
Before touching code, capture its current behavior even if it's buggy. These tests document what the code actually does, not what it should do.
// Example: Testing a messy payment processor
describe('processPayment (legacy behavior)', () => {
it('returns success for valid cards', async () => {
const result = await processPayment({
amount: 100,
card: '4242424242424242'
});
// Document actual behavior, even if weird
expect(result.status).toBe('success');
expect(result.fee).toBe(2.9); // Hardcoded fee discovered
});
it('throws on expired cards (weird: no error code)', async () => {
// This is wrong behavior, but it's what exists
await expect(
processPayment({ card: 'expired' })
).rejects.toThrow('Card error');
});
});
Why this works: You need a baseline to know if refactoring broke something. These tests prevent silent behavior changes.
Expected: Tests pass, documenting current behavior including bugs.
If it fails:
- Can't test because too many dependencies: Use Step 2 first to extract logic
- Test setup takes 200 lines: Your code has too many side effects, note this for later
Step 2: Extract Pure Functions
Find pure logic (no I/O, no mutations) and extract it. This creates testable units and reduces coupling.
// Before: Spaghetti mixing logic and side effects
async function processOrder(orderId) {
const order = await db.getOrder(orderId);
let total = 0;
for (let item of order.items) {
if (item.discount && item.discount.active) {
total += item.price * (1 - item.discount.percent / 100);
} else {
total += item.price;
}
}
if (order.coupon) {
total = total * 0.9;
}
await emailService.send(order.email, `Total: $${total}`);
return total;
}
// After: Extract pure calculation
function calculateOrderTotal(items, coupon) {
// Pure function: same inputs = same output
let total = items.reduce((sum, item) => {
const price = item.discount?.active
? item.price * (1 - item.discount.percent / 100)
: item.price;
return sum + price;
}, 0);
return coupon ? total * 0.9 : total;
}
async function processOrder(orderId) {
const order = await db.getOrder(orderId);
const total = calculateOrderTotal(order.items, order.coupon);
await emailService.send(order.email, `Total: $${total}`);
return total;
}
Why this works: Pure functions are easy to test and reason about. You can verify the math works without database or email setup.
Expected: Original tests still pass, new pure function tests are simple.
If it fails:
- Function needs too many parameters: Sign of poor data structure, refactor later
- Can't extract because everything mutates state: Start with Step 3 instead
Step 3: Introduce Parameter Objects
When functions take 5+ parameters or you see the same group passed around, bundle them into objects. This reduces coupling and makes signatures manageable.
// Before: Too many parameters
function createInvoice(
customerId,
items,
taxRate,
discountCode,
billingAddress,
shippingAddress,
paymentMethod,
currency
) {
// 100 lines of logic
}
// After: Group related data
interface InvoiceRequest {
customer: {
id: string;
billingAddress: Address;
shippingAddress: Address;
};
order: {
items: LineItem[];
discountCode?: string;
};
payment: {
method: PaymentMethod;
currency: string;
};
taxRate: number;
}
function createInvoice(request: InvoiceRequest) {
// Same logic, clearer intent
const subtotal = calculateSubtotal(request.order.items);
const discount = applyDiscount(subtotal, request.order.discountCode);
// ...
}
Why this works: Objects group related data, making dependencies explicit. You can add fields without changing every call site.
Expected: Function signatures get shorter, tests become more readable.
If it fails:
- Not sure how to group parameters: Look for params that change together
- Object becomes a dumping ground: You may need multiple objects for different concerns
Step 4: Replace Nested Conditionals with Early Returns
Deeply nested if/else blocks hide the happy path. Invert conditions and return early to flatten logic.
// Before: Nested conditionals
function approveRefund(order, reason) {
if (order) {
if (order.status === 'delivered') {
if (order.daysSinceDelivery <= 30) {
if (reason === 'defective' || reason === 'wrong-item') {
// Actual logic buried 4 levels deep
return processRefund(order);
} else {
return { error: 'Invalid reason' };
}
} else {
return { error: 'Too late' };
}
} else {
return { error: 'Not delivered' };
}
} else {
return { error: 'Order not found' };
}
}
// After: Early returns flatten structure
function approveRefund(order, reason) {
// Guard clauses make requirements explicit
if (!order) {
return { error: 'Order not found' };
}
if (order.status !== 'delivered') {
return { error: 'Not delivered' };
}
if (order.daysSinceDelivery > 30) {
return { error: 'Too late' };
}
const validReasons = ['defective', 'wrong-item'];
if (!validReasons.includes(reason)) {
return { error: 'Invalid reason' };
}
// Happy path is now obvious
return processRefund(order);
}
Why this works: Each condition is a clear gate. Readers can scan requirements without mental stack management.
Expected: Function reads top-to-bottom, main logic is at the end.
If it fails:
- Some conditions need to combine: That's fine, just keep nesting under 2 levels
- Lots of early returns feel wrong: This is idiomatic in modern code, especially TypeScript/Rust
Step 5: Run All Tests After Each Change
After every refactoring step, run your full test suite. This is non-negotiable.
# After each refactor
npm test
# If you have integration tests
npm run test:integration
# Check types if using TypeScript
npm run type-check
Expected: All tests pass with same output as before refactoring.
If it fails:
- Tests fail: Revert last change immediately, refactor smaller
- Tests take too long: Run unit tests first, integration tests before commit
- No tests exist: Go back to Step 1, you're refactoring blind
Verification
Run your tests and manually verify critical paths:
# Full test suite
npm test
# Manual smoke test
npm start
# Navigate to the refactored feature
# Try the most common user flows
You should see: All tests green, application behaves identically to before.
What You Learned
Refactoring safely requires tests first, small steps, and constant verification. Extract pure functions before restructuring control flow. Stop when code is "good enough" for the current task.
Limitations:
- This adds time upfront but prevents production bugs
- Some code is too coupled to refactor safely without bigger changes
- Don't refactor code you're not actively changing
When NOT to refactor:
- Code works fine and you're not modifying it
- Under extreme time pressure (mark with TODO instead)
- The whole system needs redesign (incremental refactoring won't help)
Refactoring Checklist
Before starting:
- Can you explain what the code does?
- Do characterization tests exist?
- Have you committed working code?
- Can you revert changes easily?
During refactoring:
- Make one change at a time
- Run tests after each change
- Keep commits small and focused
- Don't add features while refactoring
After refactoring:
- All tests pass
- Code review completed
- Manually tested critical paths
- Documentation updated if needed
Common Patterns to Refactor
Long Parameter Lists → Parameter Object
// Before: 7 parameters
function sendEmail(to, from, subject, body, cc, bcc, attachments)
// After: 1 object
function sendEmail(email: EmailMessage)
Repeated Null Checks → Optional Chaining
// Before
if (user && user.profile && user.profile.settings) {
return user.profile.settings.theme;
}
// After
return user?.profile?.settings?.theme ?? 'default';
Magic Numbers → Named Constants
// Before
if (order.total > 1000) { ... }
// After
const FREE_SHIPPING_THRESHOLD = 1000;
if (order.total > FREE_SHIPPING_THRESHOLD) { ... }
Duplicated Logic → Extract Function
// Before: Same calculation in 3 places
const discount1 = price * 0.1;
const discount2 = price * 0.1;
// After: Single source of truth
function calculateDiscount(price) {
return price * 0.1;
}
Real-World Example
Here's a complete before/after from a production codebase:
// BEFORE: 80 lines, hard to test, multiple responsibilities
async function handleCheckout(req, res) {
const userId = req.session.userId;
if (!userId) {
return res.status(401).json({ error: 'Not logged in' });
}
const user = await db.users.findById(userId);
if (!user) {
return res.status(404).json({ error: 'User not found' });
}
const cart = await db.carts.findByUserId(userId);
if (!cart || cart.items.length === 0) {
return res.status(400).json({ error: 'Cart empty' });
}
let total = 0;
for (let item of cart.items) {
const product = await db.products.findById(item.productId);
if (item.quantity > product.stock) {
return res.status(400).json({
error: `Not enough ${product.name}`
});
}
total += product.price * item.quantity;
}
if (req.body.coupon) {
const coupon = await db.coupons.findByCode(req.body.coupon);
if (coupon && coupon.active) {
total = total * (1 - coupon.discount);
}
}
const charge = await stripe.charges.create({
amount: total * 100,
currency: 'usd',
customer: user.stripeId
});
const order = await db.orders.create({
userId,
items: cart.items,
total,
chargeId: charge.id
});
await db.carts.deleteByUserId(userId);
await emailService.sendReceipt(user.email, order);
return res.json({ orderId: order.id });
}
// AFTER: Separated concerns, testable, clear flow
async function handleCheckout(req, res) {
const userId = req.session.userId;
if (!userId) {
return res.status(401).json({ error: 'Not logged in' });
}
try {
const order = await processCheckout(userId, req.body.coupon);
return res.json({ orderId: order.id });
} catch (error) {
return handleCheckoutError(error, res);
}
}
async function processCheckout(userId, couponCode) {
// Each step is now a testable function
const user = await requireUser(userId);
const cart = await requireNonEmptyCart(userId);
await validateInventory(cart.items);
const pricing = calculatePricing(cart.items, couponCode);
const charge = await chargeCustomer(user.stripeId, pricing.total);
const order = await createOrder({
userId,
items: cart.items,
pricing,
chargeId: charge.id
});
await Promise.all([
clearCart(userId),
sendReceiptEmail(user.email, order)
]);
return order;
}
// Pure function: easy to test
function calculatePricing(items, couponCode) {
const subtotal = items.reduce(
(sum, item) => sum + item.price * item.quantity,
0
);
const discount = getCouponDiscount(couponCode, subtotal);
const total = subtotal - discount;
return { subtotal, discount, total };
}
// Helper functions with single responsibilities
async function requireUser(userId) {
const user = await db.users.findById(userId);
if (!user) throw new NotFoundError('User not found');
return user;
}
async function requireNonEmptyCart(userId) {
const cart = await db.carts.findByUserId(userId);
if (!cart || cart.items.length === 0) {
throw new ValidationError('Cart empty');
}
return cart;
}
function handleCheckoutError(error, res) {
if (error instanceof NotFoundError) {
return res.status(404).json({ error: error.message });
}
if (error instanceof ValidationError) {
return res.status(400).json({ error: error.message });
}
// Log unexpected errors
console.error('Checkout failed:', error);
return res.status(500).json({ error: 'Checkout failed' });
}
What improved:
- Main handler is now 10 lines instead of 80
- Pure
calculatePricingfunction is trivial to test - Each step has a clear name and single responsibility
- Error handling is centralized
- Async operations run in parallel where possible
- Database and Stripe mocking now only needed in integration tests
Tested with Node.js 22.x, TypeScript 5.5, Jest 29.x