Functions and side effects: guard clauses, no flags, a pure core Intermediate

This page answers: what refactor commits 1–3 from the commit sequence look like in code, and why each step makes the next change cheaper.

Loading diagram…

Guard clauses instead of nested ifs

The matching refactoring in Fowler’s catalog: Replace Nested Conditional with Guard Clauses. Exceptional cases exit early, and the main path hugs the left margin.

// Before: the main path sits at the third indentation level
if (items && items.length > 0) {
  if (user) { /* ... 20 lines ... */ } else throw new Error('error');
}
return 0;

// After
if (cmd.lines.length === 0) return { ok: false, error: 'EMPTY_CART' };
const customer = await this.orders.findCustomer(cmd.userId);
if (!customer) return { ok: false, error: 'CUSTOMER_NOT_FOUND' };
// main path from here on, no indentation

One level of abstraction per function

A function that both calls a business-level placeOrder and concatenates SQL strings is mixing two levels. The reader has to keep changing “zoom”. An orchestrating function should only call named steps; the details live inside those steps.

export function quoteTotalVnd(input: QuoteInput, now: Date): number {
  if (input.lines.length === 0) return 0;

  const subtotal = subtotalVnd(input.lines);
  const discounted = applyCoupon(subtotal, input.couponCode, now);
  return discounted + shippingFeeVnd(discounted, input.shipping);
}

This is Extract Function. Stop extracting when the new function’s name says nothing more than its body; that is exactly where Ousterhout objects to over-splitting.

Drop boolean flags, group parameters

Fowler calls these Remove Flag Argument and Introduce Parameter Object.

Before	After	Reason
dryRun: boolean	Two methods: quote() and placeOrder()	Two different behaviors (with/without side effects) deserve two names
express: boolean	shipping: 'standard' | 'express'	Adding same_day doesn’t turn the signature into two booleans
5 loose parameters	PlaceOrderCommand	The call site reads by field name

export type ShippingMethod = 'standard' | 'express';

export interface QuoteInput {
  lines: readonly CartLine[];
  couponCode?: string;
  shipping: ShippingMethod;
}

export interface PlaceOrderCommand extends QuoteInput {
  userId: string;
  idempotencyKey: string;
}

// Call site: self-explanatory
await checkout.placeOrder({ userId, lines, shipping: 'express', idempotencyKey });

When the number of shipping or promotion variants grows and each has its own formula, move to Strategy. Don’t extend the union with switch statements scattered everywhere.

Magic numbers → named constants with units

Good names carry the unit: _VND, _MS, _SECONDS, _HOUR. The classic bug: TIMEOUT = 30, with half the team reading seconds and the other half milliseconds.

export const FREE_SHIPPING_THRESHOLD_VND = 500_000;
export const SHIPPING_FEE_VND = { standard: 30_000, express: 50_000 } as const;
export const VIP10_PRICE_FACTOR = 0.9;
// Hour in the process's timezone (keeps the old behavior); switching to Vietnam time is a separate commit.
export const VIP10_CUTOFF_HOUR = 12;

export type ShippingMethod = keyof typeof SHIPPING_FEE_VND;

• SHIPPING_FEE_VND is also the source of the ShippingMethod type: add a new method but forget its fee, and it won’t compile.
• Not every number needs a constant. 0, 1, index + 1 are clear in place. Matching refactoring: Replace Magic Literal.
• Business values that change often (the free-shipping threshold) belong in config or a DB table. A constant is only the first step toward having one place to change.

Side effects at the edges, a pure core

The idea of functional core, imperative shell (Gary Bernhardt):

• Core: the deciding part (computing totals, choosing states) is pure functions.
• Shell: the executing part (DB, HTTP, clock, logs) is thin and on the outside.
• In Fowler’s catalog, this step is close to Split Phase and Separate Query from Modifier.

After refactoring: the shell gathers input and performs side effects, the core only computes

Enlarge diagram

Controller → CheckoutServicePlaceOrderCommand

◀ Prev▶ PlayNext ▶↺ RestartStep 1/5

The controller validates the DTO and sends a PlaceOrderCommand (parameter object) to the service. No more express/dryRun booleans.

@Injectable()
export class CheckoutService {
  constructor(
    @Inject(ORDER_REPOSITORY) private readonly orders: OrderRepository,
    @Inject(PAYMENT_PORT) private readonly payments: PaymentPort,
    @Inject(CLOCK) private readonly clock: Clock,
  ) {}

  quote(input: QuoteInput): number {
    return quoteTotalVnd(input, this.clock.now());
  }

  async placeOrder(cmd: PlaceOrderCommand): Promise<PlaceOrderResult> {
    if (cmd.lines.length === 0) return { ok: false, error: 'EMPTY_CART' };
    const customer = await this.orders.findCustomer(cmd.userId);
    if (!customer) return { ok: false, error: 'CUSTOMER_NOT_FOUND' };

    const totalVnd = quoteTotalVnd(cmd, this.clock.now());
    const charge = await this.charge(cmd, totalVnd);
    if (charge.status === 'declined') return { ok: false, error: 'PAYMENT_DECLINED' };

    const order = await this.orders.save({ customerId: customer.id, totalVnd, paymentId: charge.paymentId });
    return { ok: true, orderId: order.id, totalVnd };
  }
}

• Measurable benefit: the golden master demo runs 10,000 cases in one ts-node process with no mocks, because quoteTotalVnd touches no I/O.
• Tests for placeOrder only need a few orchestration cases (declined, infrastructure error, success) with fake ports.
• placeOrder here already includes commit 4 (don’t save the order when the charge fails) and turns return 0 for an empty cart into a named error. Those are behavior changes, so they belong in a separate fix: commit, with the tests updated.

The charge method and the PlaceOrderResult type are explained on the next page.

Self-check: what difference between quote() and placeOrder() makes them deserve two names instead of a dryRun flag?

Next: Errors, comments, DRY