Error handling, comments, duplication and dependency direction Intermediate

This page answers: which errors should be return values and which should be exceptions; which comments to keep; and when keeping two duplicate pieces of code is the right call.

Error handling: don’t swallow, classify, add context

Loading diagram…

Kind	Example	Representation	Who handles it
Expected business branch	Empty cart, card declined, expired coupon	Typed result (ok: false)	The use case, then mapped to HTTP
Infrastructure error	DB timeout, payment gateway 503	Exception with context and cause	Exception filter, retry, alert
Bug	Unexpected undefined, broken invariant	Let it crash, don’t catch	Fix the code

export type PlaceOrderResult =
  | { ok: true; orderId: string; totalVnd: number }
  | { ok: false; error: 'EMPTY_CART' | 'CUSTOMER_NOT_FOUND' | 'PAYMENT_DECLINED' };

export class PaymentUnavailableError extends Error {}

// private method in CheckoutService
private async charge(cmd: PlaceOrderCommand, totalVnd: number) {
  try {
    return await this.payments.charge({ amountVnd: totalVnd, idempotencyKey: cmd.idempotencyKey });
  } catch (err) {
    throw new PaymentUnavailableError(
      `charge failed: user=${cmd.userId} amount=${totalVnd}VND key=${cmd.idempotencyKey}`,
      { cause: err },
    );
  }
}

• Don’t swallow: a catch is only valid if it really handles the error (retry, a deliberate fallback) or rethrows with better information.
• Add context: a raw ECONNRESET doesn’t say which order or how much money. The cause option (ES2022) keeps the original error and its stack; it needs lib ES2022 in tsconfig.
• Don’t use exceptions for expected branches: declines happen every day. Throwing means callers need try/catch just to learn a normal outcome, and TypeScript can’t check that callers handled every case.
• Classifying provider signals (declined, retryable, unknown outcome) is the adapter’s job; see the Adapter pattern.

At the HTTP edge:

• The controller maps PlaceOrderResult to a status and body per RFC 9457 Problem Details: CUSTOMER_NOT_FOUND → 404, PAYMENT_DECLINED → 422.
• PaymentUnavailableError goes through a NestJS exception filter and becomes a 503, logging the full cause without leaking internal details to the client.

Result types for expected errors

• The function signature spells out the outcomes; if a caller forgets a branch, an exhaustive switch fails to compile.
• “Card declined” doesn’t get mixed up with “database is down” in the same catch.
• Testing error branches only means comparing return values.

The cost

• Results must be passed through each layer; overusing them for infrastructure errors fills code with if (!r.ok) return r.
• There’s no syntax support like Rust’s ?; a third-party Result library is one more dependency the whole team must learn.
• Most frameworks (NestJS filters, middleware) are built around exceptions; you still need exceptions at the edge.

Comments: explain why, delete commented-out code

Comment type	Keep?	Example
Narrating the code	Delete	// loop over items adding to total
Explaining why or an external constraint	Keep	// The payment gateway dedupes by key for 24h, so the key must derive from the order, not be random on each retry
Warning about non-obvious behavior	Keep	// Hour in the process's timezone; switching to Vietnam time is a separate commit
Commented-out code (// await this.mailer...)	Delete	Git already remembers; leaving it only raises “turn it back on or not?”
TODO without a ticket	Turn into a ticket or delete	An ownerless TODO never gets done

This is where Martin and Ousterhout disagree most: Martin sees comments as “a necessary evil”, Ousterhout sees them as part of the abstraction. Pragmatic default: good names replace “what” comments; write “why” comments freely.

Duplication vs the wrong abstraction

Sandi Metz, The Wrong Abstraction (2016): “duplication is far cheaper than the wrong abstraction”, with the advice “prefer duplication over the wrong abstraction”.

Loading diagram…

• Metz’s way out: inline the abstraction back into each caller (Inline Function), have each caller delete what it doesn’t use, and only then look for the right new abstraction.
• Rule of three (popularized by Fowler in Refactoring, credited to Don Roberts): the first time just write it, the second time accept the duplication, the third time refactor. Only by the third time can you see what is truly shared.

Sign	Lean toward
Two similar pieces that change for different reasons (B2C vs B2B shipping fees)	Keep the duplication
A single business rule copied around (free-shipping threshold in 3 places)	Consolidate now: it’s one fact, not three
A shared function with a boolean parameter where each caller passes a fixed value	Wrong abstraction, inline it back

Dependency direction and testability

The core imports nothing from infrastructure. The shell depends on interfaces defined by the application side (PaymentPort, OrderRepository, Clock). NestJS DI provides implementations through tokens; tests provide fakes.

export interface Clock { now(): Date }
export const CLOCK = Symbol('CLOCK');

// module
providers: [{ provide: CLOCK, useValue: { now: () => new Date() } satisfies Clock }]

// test
const fixedClock: Clock = { now: () => new Date('2026-10-18T09:00:00+07:00') };

• Details on tokens, useClass/useValue and resolution errors: NestJS Dependency Injection.
• Quick check: if testing a function means mocking the fs module, Date or global fetch, the dependencies point the wrong way.

Code smell → refactoring table

The four smells Long Function, Long Parameter List, Primitive Obsession and Repeated Switches are names from chapter 3 of Fowler’s Refactoring (2nd edition). The other rows are this cluster’s descriptions, not smell names from the book. Refactoring names match the catalog.

Smell	In the example	Refactoring
Long Function	checkout, 30 lines, 4 levels	Extract Function, Split Phase
Long Parameter List	5 loose parameters	Introduce Parameter Object
Flag argument	express, dryRun	Remove Flag Argument
Nested conditionals	Nested ifs	Replace Nested Conditional with Guard Clauses
Magic literal	500000, 0.9	Replace Magic Literal
Primitive Obsession	Money as a bare number, coupon as a string	Replace Primitive with Object
Repeated Switches	switch (shipping) in many files	Replace Conditional with Polymorphism (Strategy)
Dead code / commented-out code	// await this.mailer...	Remove Dead Code
Query mixed with side effects	Computing the total and writing to the DB in one function	Separate Query from Modifier
Wrong abstraction	Shared function full of flags	Inline Function, then Extract again

Self-check

1. How should “card declined” and “payment gateway timeout” be represented differently, and why?
2. When is keeping two duplicate pieces of code the right decision?

Next: Common mistakes