Same Results Less Code

Community Refactoring Best Practices: Same Results, Less Code

Code-review and refactoring guide focused on the parts of code volume that come from judgment and modelling gaps — wrong abstraction choices, hidden semantic duplication, defensive habits, premature generality. This skill deliberately skips what linters and tools like knip, eslint, ruff, tsc --noUnusedLocals, or formatters already catch. It is the second pass: after the mechanical cleanup, what remains?

Core Principles

1. Preserve behaviour. Every transformation must produce identical observable behaviour — same outputs, same errors, same side effects, same API surface. 2. Earlier mistakes cascade. A wrong frame multiplies into wrong shapes, which multiply into duplicate logic. Optimise from the top of the lifecycle. 3. Explain why, not just what. Each rule explains the cost of the anti-pattern so judgment can transfer to novel cases. 4. Quantify where possible. Prefer "eliminates N lines / prevents X bug class" over "cleaner." 5. Don't over-refactor. Rule of three: extract abstractions when duplication has actually appeared three times, not in anticipation.

When to Apply

Use this skill when:

• Reviewing a PR for "could this be simpler?" (the question linters can't answer)
• Refactoring code that has grown in volume without growing in capability
• Auditing a module that "feels heavy" — many flags, many layers, many checks
• Onboarding to an unfamiliar codebase and trying to spot the parts that are accidental volume vs essential complexity
• Designing a new module and wanting to avoid the common over-abstraction traps
• Working alongside knip / eslint / ruff and wanting the layer of judgment those tools can't supply

Don't use this skill for:

• Mechanical cleanup that a linter or formatter already does (unused imports, dead exports, style) — use knip, eslint, ruff, or prettier/black instead.
• Algorithmic complexity / performance tuning — use `complexity-optimizer` for that.
• General cleanup of recently modified code regardless of mental-model gaps — use `code-simplifier`.

Rule Categories by Priority

Quick Reference

1. Reinvention (CRITICAL)

• `reinvent-stdlib-collection-ops` — Reach for .map/.filter/.reduce before writing loops
• `reinvent-date-and-time` — Stop hand-rolling date and time arithmetic
• `reinvent-deep-equality` — Use a real deep-equal instead of hand-recursing objects
• `reinvent-explicit-state-machine` — Surface a state machine instead of boolean flag juggling
• `reinvent-builtin-data-structures` — Recognise when a custom container is just a Map, Set, or Queue

2. Wrong Frame (CRITICAL)

• `frame-function-not-class` — Use a function when the class has no identity
• `frame-manager-noun-is-a-verb` — Rename Manager/Helper/Util classes until the real verb appears
• `frame-composition-over-inheritance-for-shared-fields` — Compose shared fields instead of inheriting
• `frame-data-over-procedure` — Model the problem as data before writing procedure
• `frame-monolith-by-cohesive-axis` — Split a god-function along its cohesive axis, not by line count

3. Hidden Duplication (HIGH)

• `dup-parallel-types-same-shape` — Collapse parallel types that share a shape
• `dup-near-twin-functions` — Parameterize two functions that differ by a literal
• `dup-mirrored-branches` — Lift shared lines out of mirrored if/else branches
• `dup-config-not-copies` — Replace many hardcoded copies with one table
• `dup-cross-layer-shape` — Collapse identical DTOs, DB rows, and domain objects

4. Derived State Stored (HIGH)

• `derive-dont-store-computed` — Compute what you can compute; store only what you can't
• `derive-single-source-of-truth` — Pick one source of truth; derive the rest
• `derive-boolean-from-data` — Derive booleans from the data, don't track them separately
• `derive-cache-as-getter-not-field` — Turn cached fields into getters until profiling proves otherwise
• `derive-url-as-state` — Let the URL or route be the state, not a mirror of it

5. Procedural Rebuilds (MEDIUM-HIGH)

• `proc-mutation-builder-over-pipeline` — Compose pipelines when the mutation-builder hides the intent
• `proc-if-chain-as-lookup` — Replace if/elif returning constants with a lookup table
• `proc-manual-recursion-of-walk` — Use a recognised tree/object walk, not hand-coded recursion
• `proc-build-vs-declarative-template` — Use the declarative form when the framework provides one
• `proc-sequential-awaits-could-be-parallel` — Parallelise independent awaits

6. Speculative Generality (MEDIUM)

• `spec-interface-of-one` — Avoid defining an interface for a single implementation
• `spec-options-bag-of-one` — Avoid options bags where every caller passes the same values
• `spec-flag-driven-paths` — Split a function that a boolean flag has made into two
• `spec-no-extension-point-without-extender` — Delete extension points that have no second user
• `spec-generic-over-one-type` — Drop the generic parameter when only one concrete type uses it

7. Defensive Excess (MEDIUM)

• `defense-guard-against-impossible` — Stop guarding against states the type/flow already rules out
• `defense-validate-once-at-boundary` — Validate once at the boundary, trust inside
• `defense-let-it-throw` — Let exceptions propagate; don't catch what you can't handle
• `defense-null-pollution-from-bad-modelling` — Fix the type that makes the null checks necessary

8. Type System Underuse (LOW-MEDIUM)

• `types-discriminated-union-over-flags` — Use a discriminated union instead of optional fields + tags
• `types-literal-union-over-string` — Narrow string down to a literal union when the set is closed
• `types-no-any-to-silence` — Avoid reaching for any/as to silence a type error
• `types-branding-over-runtime-checks` — Brand a validated value so you don't validate it twice
• `types-exhaustive-switch-not-default` — Use exhaustiveness checks instead of a catch-all default
• `types-readonly-and-immutable-by-default` — Mark data readonly until mutation is actually needed

How to Apply (Workflow)

When asked to review or refactor code with this skill:

1. Run the mechanical pass first. knip/eslint/ruff/tsc --noUnusedLocals will catch dead code, unused imports, style. Don't duplicate that work here. 2. *Read the file or PR for intent. Ask: what is this code trying to do? The judgment skill is recognising when the implementation overshoots the intent. 3. Walk the categories in priority order.*

• Start with Reinvention and Frame — the biggest wins live there.
• Then Duplication and Derived state.
• Then Procedural rebuilds and Speculative generality.
• Defensive and type-system issues last — they're high frequency but localised.

4. Propose minimal-diff transformations. Each rule shows incorrect → correct as a tight diff; preserve that property in suggestions. 5. Verify behaviour. Outputs, errors, and side effects must be identical. Tests must still pass. 6. Don't bundle unrelated changes. Each transformation should map to one category. Mixing them makes the change hard to review.

When NOT to Apply

• Code is younger than the rule of three (one or two duplicates) — extracting is premature.
• The pattern is genuinely a known exception (see each rule's "When NOT to use this pattern" section).
• The refactor would be a large, risky rewrite without a clear test safety net — propose, don't execute.
• Performance-critical hot paths where the "simpler" form has measurable cost — measure first.

Reference Files

Related Skills

• `code-simplifier` — Mechanical simplification (naming, dead code, nesting). Complementary first pass.
• `complexity-optimizer` — Algorithmic/performance complexity. Different axis.
• `refactor` — General-purpose refactoring workflow.
• `clean-code` — Broader clean-code principles. This skill is the narrower, judgment-focused subset.

Refactoring

Version 0.1.0 Community May 2026

Note:

This document is mainly for agents and LLMs to follow when refactoring, maintaining,

generating, or refactoring codebases. Humans may also find it useful,

but guidance here is optimized for automation and consistency by AI-assisted workflows.

---

Abstract

Code-review and refactoring guide focused on the judgment gaps that produce excess code volume — wrong abstraction frame, hidden semantic duplication, derived state stored as state, procedural rebuilds of declarative concepts, speculative generality, defensive checks against impossible states, and type-system underuse. Contains 40 rules across 8 categories, prioritized by cascade impact. Deliberately skips what linters and tools like knip, eslint, ruff, and tsc already catch; this is the second pass that operates on conceptual modelling rather than mechanical cleanup. Each rule includes a WHY explanation, incorrect-vs-correct code examples with minimal diffs, and clear 'when NOT to apply' guidance.

---

Table of Contents

1. Reinvention — CRITICAL

• 1.1 Reach for Standard Collection Operations Before Writing Loops — CRITICAL (eliminates index variables and accumulator bugs; reduces 5-20 line loops to one expression)
• 1.2 Recognise When a Custom Container Is Just a Map, Set, or Queue — CRITICAL (eliminates 50-200 line wrapper classes around Map, Set, or Deque)
• 1.3 Stop Hand-Rolling Date and Time Arithmetic — CRITICAL (prevents DST and locale bugs; reduces 20-60 lines of date math to 1-3)
• 1.4 Surface an Explicit State Machine Instead of Boolean Flag Juggling — CRITICAL (4-8 boolean flags collapsed to a single tagged state; eliminates impossible-state bugs)
• 1.5 Use a Real Deep-Equality or Hash Instead of Hand-Recursing Objects — CRITICAL (30-100 lines of recursive comparison reduced to a single library call)

2. Wrong Frame — CRITICAL

• 2.1 Compose Shared Fields Instead of Inheriting From a Base Class — CRITICAL (eliminates rigid multi-level class hierarchies in favour of intersected types)
• 2.2 Model the Problem as Data Before Writing Procedure — CRITICAL (reduces 50-200 lines of branches to a small table plus one interpreter)
• 2.3 Rename Manager/Helper/Util Classes Until the Real Verb Appears — CRITICAL (reduces grab-bag "Manager" classes to focused functions in their real modules)
• 2.4 Split a God-Function Along Its Cohesive Axis, Not by Line Count — CRITICAL (reduces a 300-line procedure to 3-5 independently testable pieces)
• 2.5 Use a Function When the Class Has No Identity — CRITICAL (eliminates 50-100 lines of class ceremony around a stateless function)

3. Hidden Duplication — HIGH

• 3.1 Collapse Identical DTOs, DB Rows, and Domain Objects — HIGH (eliminates a layer of pass-through mappers and the entity-x3 type explosion)
• 3.2 Collapse Parallel Types That Share a Shape — HIGH (eliminates three near-identical types and their mappers (~100 lines))
• 3.3 Lift Shared Lines Out of Mirrored Branches — HIGH (reduces twin if/else bodies to one shared block plus the actual difference)
• 3.4 Parameterize Two Functions That Differ by a Literal — HIGH (eliminates a copy-paste twin function and the diff-rot bug class)
• 3.5 Replace Many Hardcoded Copies With One Table — HIGH (reduces N copy-pasted definitions to 1 table; eliminates the "I forgot to update one" bug)

4. Derived State Stored — HIGH

• 4.1 Compute What You Can Compute; Store Only What You Can't — HIGH (eliminates state variables and the sync bugs they cause)
• 4.2 Derive Booleans From the Data, Don't Track Them Separately — HIGH (eliminates one boolean of state per "is X?" question (and its sync code))
• 4.3 Let the URL or Route Be the State, Not a Mirror of It — HIGH (eliminates URL-vs-local-state desync and the listener code that papers over it)
• 4.4 Pick One Source of Truth; Derive the Rest — HIGH (prevents two-state-sync bugs and eliminates parallel updates)
• 4.5 Turn Cached Fields Into Getters Until Profiling Proves Otherwise — HIGH (eliminates invalidation bugs and the "stale cache" class of failures)

5. Procedural Rebuilds — MEDIUM-HIGH

• 5.1 Compose Pipelines When the Mutation-Builder Hides the Intent — MEDIUM-HIGH (reduces 10-20 line accumulator-builder blocks to a 3-5 line composed pipeline)
• 5.2 Parallelise Independent Awaits — MEDIUM-HIGH (faster wall-clock time by N for N independent I/O calls; eliminates accidental serial chains)
• 5.3 Replace an if/elif Chain That Returns Different Constants With a Lookup — MEDIUM-HIGH (reduces an N-branch if/elif to a single Map or object lookup)
• 5.4 Use a Recognised Tree/Object Walk Instead of Hand-Coded Recursion — MEDIUM-HIGH (eliminates hand-rolled recursive descent with its accumulator and base-case bugs)
• 5.5 Use the Declarative Form When the Framework Provides One — MEDIUM-HIGH (eliminates imperative DOM/string builders in favour of the framework's template form)

6. Speculative Generality — MEDIUM

• 6.1 Avoid Defining an Interface for a Single Implementation — MEDIUM (eliminates one-implementer interfaces and the indirection layer they impose)
• 6.2 Avoid Options Bags Where Every Caller Passes the Same Values — MEDIUM (eliminates options-object plumbing for parameters that have one value)
• 6.3 Delete Extension Points That Have No Second User — MEDIUM (eliminates hook/plugin/registry machinery that no one extends)
• 6.4 Drop the Generic Parameter When Only One Concrete Type Uses It — MEDIUM (eliminates one-type generic indirection; reduces 5-10 lines of type plumbing)
• 6.5 Split a Function That a Boolean Flag Has Made Into Two — MEDIUM (eliminates flag-driven branching that hides two distinct functions inside one)

7. Defensive Excess — MEDIUM

• 7.1 Fix the Type That Makes the Null Checks Necessary — MEDIUM (eliminates cascading null checks by modelling the actual cases)
• 7.2 Let Exceptions Propagate; Don't Catch What You Can't Handle — MEDIUM (eliminates pass-through try/catch blocks that obscure failures)
• 7.3 Stop Guarding Against States the Type or Flow Already Rules Out — MEDIUM (eliminates defensive checks for states the type system guarantees impossible)
• 7.4 Validate Once at the Boundary, Trust Inside — MEDIUM (eliminates re-validation at every internal call; reduces N defensive checks to 1)

8. Type System Underuse — LOW-MEDIUM

• 8.1 Avoid Reaching for any/as to Silence a Type Error — LOW-MEDIUM (prevents silent type-error suppression; eliminates 5-10 lines of compensating runtime code)
• 8.2 Brand a Validated Value So You Don't Validate It Twice — LOW-MEDIUM (eliminates re-validation of values that have already been checked)
• 8.3 Mark Data Readonly Until Mutation Is Actually Needed — LOW-MEDIUM (eliminates defensive copies and prevents accidental mutation bugs)
• 8.4 Narrow string Down to a Literal Union When the Set Is Closed — LOW-MEDIUM (eliminates runtime string-comparison guards; enables compiler-checked exhaustiveness)
• 8.5 Use a Discriminated Union Instead of Optional Fields and Runtime Tags — LOW-MEDIUM (eliminates manual tag checks; reduces 5-10 lines of guards to a switch)
• 8.6 Use Exhaustiveness Checks Instead of a Catch-All default — LOW-MEDIUM (prevents silent fall-through; eliminates the "I added a case and forgot to handle it" bug class)

---

References

1. https://web.stanford.edu/~ouster/cgi-bin/aposd.php 2. https://tidyfirst.substack.com/ 3. https://refactoring.com/ 4. https://blog.janestreet.com/effective-ml-revisited/ 5. https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/ 6. https://martinfowler.com/bliki/Yagni.html 7. https://verraes.net/2019/12/speculative-generality/ 8. https://react.dev/learn/you-might-not-need-an-effect 9. https://www.typescriptlang.org/docs/handbook/2/narrowing.html 10. https://pragprog.com/titles/swdddf/domain-modeling-made-functional/ 11. https://effectivetypescript.com/ 12. https://en.wikipedia.org/wiki/Composition_over_inheritance

---

Source Files

This document was compiled from individual reference files. For detailed editing or extension:

{Imperative Title — copy from the title field}

{1-3 sentences explaining WHY this matters. State the cost of the anti-pattern: what goes wrong, what cascades, what the maintenance burden is. Avoid hedging — the reader should leave understanding the principle so they can apply it to novel cases.}

Incorrect ({short label — what's wrong}):

{Production-realistic code that exhibits the anti-pattern. Annotate the bad parts with
// comments that show the cost.}

Correct ({short label — what's right}):

{The cleaned-up version. Make it a minimal diff from the incorrect form so the
transformation is visible. Annotate with // comments showing the win.}

{If multiple legitimate approaches exist, use:}

Correct (option A — {description}):

{Option A code}

Correct (option B — {description}):

{Option B code}

{Optional sections — include the ones that help:}

Symptoms:

• {Observable signal that this anti-pattern is present}
• {Another signal}

When NOT to use this pattern:

• {A legitimate case where the anti-pattern is actually correct, with the reason}
• {Another exception}

Variations / related patterns:

• {Sibling pattern}
• {Related rule with a [[link]]}

Reference: {Title}

---

Notes for skill authors

This skill targets judgment gaps, not lint-able mechanical issues. When adding a rule:

1. Check it's not lint-able. Could knip, eslint, ruff, tsc, or prettier catch this? If yes, the rule belongs in a different skill (e.g. code-simplifier). 2. Check it's not algorithmic. Performance-complexity rules go in complexity-optimizer. 3. *The fix should require judgment about intent, modelling, or framing — not just pattern matching. 4. Show the cascade. The strongest rules explain why this anti-pattern multiplies downstream cost. 5. Include "When NOT to use this pattern."* Every rule has a legitimate exception. Naming it is what turns a rigid rule into transferable judgment.

The eight category prefixes are fixed: reinvent-, frame-, dup-, derive-, proc-, spec-, defense-, types-. If a new rule doesn't fit one of these, the category structure may need to evolve before the rule lands — open a discussion before adding a ninth prefix.

Same Results, Less Code

A code-review and refactoring skill focused on the parts of code volume that come from judgment and modelling gaps — the things a senior reviewer notices that linters can't. Contains 40 rules across 8 categories.

Overview

This skill targets the second-pass refactoring work: what remains after knip, eslint, ruff, tsc --noUnusedLocals, and formatters have done their job. It is deliberately not a clone of those tools — every rule here requires judgment about intent, modelling, or framing.

Core principles:

• Preserve behaviour. Every transformation must produce identical observable behaviour.
• Earlier mistakes cascade. Wrong frame multiplies into wrong shapes, which multiply into duplicate logic.
• Explain why, not just what. Each rule explains the cost of the anti-pattern so the model can transfer the judgment to novel cases.
• Don't over-refactor. Rule of three: extract abstractions when duplication has actually appeared three times, not in anticipation.

Structure

same-results-less-code/
├── SKILL.md                    # Entry point with quick reference
├── README.md                   # This file
├── AGENTS.md                   # Auto-built TOC navigation
├── metadata.json               # Version, organization, references
├── references/
│   ├── _sections.md            # Category definitions and ordering
│   ├── reinvent-*.md           # Reinvention rules (5, CRITICAL)
│   ├── frame-*.md              # Wrong Frame rules (5, CRITICAL)
│   ├── dup-*.md                # Hidden Duplication rules (5, HIGH)
│   ├── derive-*.md             # Derived State Stored rules (5, HIGH)
│   ├── proc-*.md               # Procedural Rebuilds rules (5, MEDIUM-HIGH)
│   ├── spec-*.md               # Speculative Generality rules (5, MEDIUM)
│   ├── defense-*.md            # Defensive Excess rules (4, MEDIUM)
│   └── types-*.md              # Type System Underuse rules (6, LOW-MEDIUM)
└── assets/
    └── templates/
        └── _template.md        # Template for new rules

Getting Started

# No installation required — this is a documentation-only skill.
# For development/validation:
pnpm install   # Install validation dependencies (optional)
pnpm build     # Build/compile AGENTS.md from source rules
pnpm validate  # Validate skill structure and content

1. Read SKILL.md for an overview and quick reference. 2. Check references/_sections.md to understand category priorities. 3. Reference individual rules as needed during code review or refactoring.

Creating a New Rule

1. Confirm the rule isn't something a linter already catches (knip, eslint, ruff, tsc). If a linter handles it, the rule belongs in a different skill (e.g. code-simplifier). 2. Confirm the fix requires judgment about intent, modelling, or framing — not just pattern matching. 3. Copy assets/templates/_template.md to references/{prefix}-{slug}.md. 4. Fill in frontmatter: title, impact, impactDescription, tags. 5. Write the WHY explanation (1-3 sentences explaining the cost of the anti-pattern). 6. Add Incorrect and Correct code examples (production-realistic, minimal-diff). 7. Add a When NOT to use this pattern section — every rule has a legitimate exception. 8. Update SKILL.md's quick reference section. 9. Rebuild AGENTS.md: node .../build-agents-md.js .

Rule File Structure

Each rule file follows this format:

---
title: Rule Title
impact: CRITICAL|HIGH|MEDIUM-HIGH|MEDIUM|LOW-MEDIUM|LOW
impactDescription: Quantified impact (e.g., "reduces 50-200 lines of branches to a small table")
tags: prefix, technique, related-concepts
---

## Rule Title

WHY this matters (1-3 sentences explaining the cost of the anti-pattern).

**Incorrect (what's wrong):**
\`\`\`typescript
Bad example
\`\`\`

**Correct (what's right):**
\`\`\`typescript
Good example - minimal diff from incorrect
\`\`\`

**When NOT to use this pattern:**

- Exception 1 with reason
- Exception 2 with reason

Reference: [Source](https://example.com)

File Naming Convention

• Prefix must match category: reinvent-, frame-, dup-, derive-, proc-, spec-, defense-, types-
• Slug should be descriptive: function-not-class, parallel-types-same-shape, dont-store-computed
• Examples: frame-function-not-class.md, dup-near-twin-functions.md, defense-let-it-throw.md

Impact Levels

Scripts

Validate the skill structure and content:

node ~/.claude/plugins/cache/dot-claude/dev-skill/*/scripts/validate-skill.js ./same-results-less-code

Rebuild AGENTS.md after adding or modifying rules (never edit AGENTS.md manually):

node ~/.claude/plugins/cache/dot-claude/dev-skill/*/scripts/build-agents-md.js ./same-results-less-code

Contributing

1. Follow the rule template exactly. 2. Ensure the first tag matches the file prefix. 3. Use production-realistic code examples (no foo/bar/baz). 4. Make the Incorrect → Correct diff minimal — preserve variable names and structure where possible. 5. Quantify impact: prefer "eliminates N lines / prevents X bug class" over "cleaner." 6. Include a When NOT to use this pattern section. Every rule has a legitimate exception, and naming it is what turns a rigid rule into transferable judgment. 7. Run validation before submitting.

Related skills

Sections

This file defines all sections, their ordering, impact levels, and descriptions. The section ID (in parentheses) is the filename prefix used to group rules.

These categories deliberately skip what linters and tools like knip, eslint, ruff, tsc --noUnusedLocals, or formatters already catch. The focus is on the parts of code volume that come from judgment and modeling gaps — the things a senior engineer notices at review that no static analyser does.

---

1. Reinvention (reinvent)

Impact: CRITICAL Description: Code written to do what the platform, language, or a one-line stdlib call already does. Each instance is dozens of lines that could be one, and each one cascades into needing its own tests, naming, edge cases, and review attention.

2. Wrong Frame (frame)

Impact: CRITICAL Description: The wrong abstraction shape for the problem — a class where a function fits, a "Manager"/"Helper" noun hiding a verb, inheritance where composition fits, an interface with one implementer. Choosing the wrong frame multiplies every later decision against it.

3. Hidden Duplication (dup)

Impact: HIGH Description: Semantically identical code wearing different names — parallel types with relabeled fields, near-twin functions that differ by one literal, mirrored if/else branches. Linters see distinct tokens; the duplication lives at the meaning level.

4. Derived State Stored (derive)

Impact: HIGH Description: State variables, cached fields, or props that hold values which can be computed from other state. Every such field is a sync bug waiting to happen and an extra invariant to maintain across all writes.

5. Procedural Rebuilds (proc)

Impact: MEDIUM-HIGH Description: Imperative reimplementation of a declarative concept — for-loops building arrays that are .map(), if/elif chains that are lookup tables, hand-coded recursion of standard tree walks. The volume comes from operating at the wrong level of abstraction.

6. Speculative Generality (spec)

Impact: MEDIUM Description: Generality built for a second user who never arrived — an interface with one implementer, an options bag with one option, a flag that splits a function into two unrelated paths, a generic over one concrete type. Each speculation pays a permanent tax.

7. Defensive Excess (defense)

Impact: MEDIUM Description: Runtime checks for states the type system or surrounding control flow already rules out — if (x === true) for known booleans, null checks after non-null narrowing, try/catch around code that cannot throw, default branches in exhaustive unions.

8. Type System Underuse (types)

Impact: LOW-MEDIUM Description: Stringly-typed values where a small enum or literal union fits, runtime tag fields where a discriminated union does it for free, casts and any used to silence a problem that the type system could solve. Code volume grows to compensate for types the engineer didn't reach for.

Stop Guarding Against States the Type or Flow Already Rules Out

Defensive code becomes noise when it checks for things that cannot happen. if (x === true) for a value typed boolean already known to be true. A null check after a non-null assertion. A try/catch around code that never throws. An else branch in a switch over a closed union. Each one is a confidence symbol — "I don't trust the types" — and each makes the code longer without ruling out a bug that was already ruled out elsewhere.

Incorrect (a small constellation of defensive noise):

function logActiveUser(user: User | null): void {
  if (user === null || user === undefined) return;
  if (!user) return;                                     // same check, again
  if (typeof user !== 'object') return;                  // the type says it's an object
  if (user.email === undefined) return;                  // email is `string`, not `string | undefined`
  if (user.email === null) return;                       // same
  if (user.status === 'active' && user.status !== 'inactive') {  // the second half is implied by the first
    if (user.active === true) {                          // `active` is `boolean`, already known to be true
      console.log(user.email);
    }
  }
}

Correct (rely on the type system; only check what's actually uncertain):

function logActiveUser(user: User | null): void {
  if (!user) return;
  if (user.status === 'active') console.log(user.email);
}
// Two checks. Both ask real questions: is the user there, and is the user active.
// The rest was paranoia about states the type system already rules out.

Common cases:

Exhaustiveness — the right way to be paranoid:

type Shape = { kind: 'circle'; r: number } | { kind: 'square'; side: number };

function area(s: Shape): number {
  switch (s.kind) {
    case 'circle': return Math.PI * s.r ** 2;
    case 'square': return s.side ** 2;
    default: {
      const _exhaustive: never = s;  // compile error if a new kind is added without a case
      return _exhaustive;
    }
  }
}
// The default isn't defensive against runtime; it's a compile-time alarm.
// Far stronger than `throw new Error('unknown shape')` at runtime.

When NOT to use this pattern:

• You're at a trust boundary — parsing user input, deserialising network/disk data, FFI calls into untyped code. The type system can't help; defensive checks are the right answer. (See `types-validation-at-boundary` for how to do this once and narrow.)
• The type system says a value is non-null but you're inside an any/unknown cast you can't avoid — keep the check, and consider whether the cast is the real bug.
• A library author writing a public API that consumers might call wrong from JS — defensive checks earn their keep as friendly errors. (Add them at the boundary, not in every internal call.)

Reference: TypeScript Handbook — Narrowing; Make Illegal States Unrepresentable

Let Exceptions Propagate; Don't Catch What You Can't Handle

A try/catch is a place where the program says, "I have a plan if this fails." When the catch block re-throws, logs and continues with undefined, returns null, or rewraps the error in a less informative one, the code is pretending to handle the error while actually swallowing or laundering it. The right form is to catch at the layer that can do something — typically the top of a request, a background job, or a UI boundary — and let everything below propagate.

Incorrect (pass-through try/catch at every layer):

async function getUser(id: string): Promise<User | null> {
  try {
    return await db.users.findUnique({ where: { id } });
  } catch (err) {
    console.error(err);
    return null;                                          // caller can't tell "not found" from "DB down"
  }
}

async function getUserName(id: string): Promise<string | null> {
  try {
    const user = await getUser(id);
    return user?.name ?? null;
  } catch (err) {
    console.error(err);                                   // catching what cannot throw — getUser already swallowed
    return null;
  }
}

async function renderUserPage(id: string): Promise<string> {
  try {
    const name = await getUserName(id);
    return name ? `<h1>${name}</h1>` : '<h1>Not found</h1>';   // 'Not found' for DB outage too
  } catch (err) {
    console.error(err);                                   // never runs — already swallowed twice
    return '<h1>Error</h1>';
  }
}
// Three try/catches. Two are no-ops. One conflates two different failures. The actual
// failure (DB down) reaches the user as "Not found." Operators get console noise, not alerts.

Correct (catch at the boundary that has a plan; trust above):

async function getUser(id: string): Promise<User | null> {
  return db.users.findUnique({ where: { id } });
  // Returns null when not found (the DB's contract).
  // Throws when the DB is unreachable — and that's the right answer for callers.
}

async function getUserName(id: string): Promise<string | null> {
  const user = await getUser(id);
  return user?.name ?? null;
}

async function renderUserPage(id: string): Promise<string> {
  try {
    const name = await getUserName(id);
    return name ? `<h1>${name}</h1>` : '<h1>Not found</h1>';
  } catch (err) {
    logger.error({ err, id }, 'render_user_page_failed');
    metrics.increment('page_error');
    return '<h1>Sorry, something went wrong.</h1>';
  }
}
// One try/catch — at the boundary that can decide between "user not found" (a real outcome)
// and "system error" (an alarm). The rest of the stack stays clean.

Telltale anti-patterns:

• try { … } catch (e) { throw e; } — does literally nothing.
• try { … } catch (e) { throw new Error(e.message); } — destroys the stack trace; doesn't add information.
• try { … } catch { return undefined; } — turns "actual error" into "caller's edge case." Two failure modes now look identical.
• try { … } catch (e) { console.error(e); } and then continues with garbage — the program is in a broken state but acts like everything's fine.
• catch (e) { /* TODO: handle */ } — three years old, still TODO. Either delete the catch or actually handle.

Where catches earn their keep:

• HTTP boundary. Translate exceptions into 4xx/5xx responses; log; emit metrics.
• Background job runner. Decide retry, dead-letter, or alert.
• UI boundary (React error boundary, Vue errorCaptured). Show a fallback; report.
• A specific recoverable case — e.g. try { JSON.parse(s) } catch { /* fall back to raw */ }. The catch is intentional and tight.

When NOT to use this pattern:

• A function that promises a value or `null` (not an exception) — wrapping a throwing call in try { … } catch { return null } is fine; it's the contract.
• Logging the error with full context before re-throwing is fine when the call site has information the boundary doesn't. But re-throw — don't swallow.

Reference: The Pragmatic Programmer — Crashing Early (Hunt & Thomas)

Fix the Type That Makes the Null Checks Necessary

When a function is half null-check by line count — if (!user) return; if (!user.profile) return; if (!user.profile.address) return; — the type is misshapen, not the function. The engineer made everything optional "just in case," and now every reader has to walk through the safety dance. Usually one of these is true: the value is always present (drop the | null), or the value's absence means a different case worth modelling (lift it into a discriminated union).

Incorrect (a path of null checks because the type was modelled lazily):

type User = {
  id: string;
  profile?: {
    name?: string;
    address?: {
      street?: string;
      city?: string;
      country?: string;
    };
  };
};

function shippingLabel(user: User): string {
  if (!user.profile) return 'No profile';
  if (!user.profile.address) return 'No address';
  if (!user.profile.address.street) return 'Incomplete address';
  if (!user.profile.address.city) return 'Incomplete address';
  if (!user.profile.address.country) return 'Incomplete address';
  return `${user.profile.address.street}, ${user.profile.address.city}, ${user.profile.address.country}`;
  // 6 lines of guarding. Three of them collapse to "incomplete address."
  // The type lets you have a profile with a half-complete address — but does the business?
}

Correct (model the actual cases; the function becomes a switch):

type Address = { street: string; city: string; country: string };

type User =
  | { kind: 'no-profile';  id: string }
  | { kind: 'no-address';  id: string; name: string }
  | { kind: 'shippable';   id: string; name: string; address: Address };

function shippingLabel(user: User): string {
  switch (user.kind) {
    case 'no-profile': return 'No profile';
    case 'no-address': return 'No address';
    case 'shippable':  return `${user.address.street}, ${user.address.city}, ${user.address.country}`;
  }
  // Three cases. Each has exactly the fields it needs.
  // The "partial address" state literally can't exist — the type rules it out.
  // The set of business states is now visible.
}

Or, when the value is genuinely "always there" in this context:

type AuthenticatedUser = {
  id: string;
  profile: { name: string; address: Address };  // no optionality
};

function shippingLabel(user: AuthenticatedUser): string {
  const { street, city, country } = user.profile.address;
  return `${street}, ${city}, ${country}`;
  // The type carries the guarantee. The check happened once, when the user authenticated.
}

Symptoms of "the type is lying about what's optional":

• A function whose first half is a chain of if (!x) return ... and second half is the real work.
• The same chain appears in multiple functions that all consume the same type.
• Optionality on fields that are always set together (you have all three or none) — that's a union of cases, not three independent optionals.
• Optionality added because "the API might not return it" — and the API has always returned it for years.

When NOT to use this pattern:

• The value really is optional, and each absence has its own meaning that the caller cares about. Then the chain is real — use optional chaining (user.profile?.address?.street ?? 'unknown') for readability.
• Modelling discriminated unions across a large codebase is genuinely too disruptive right now — defensive checks are fine as an interim. Schedule the real fix.

Reference: Make Illegal States Unrepresentable (Yaron Minsky); Parse, Don't Validate (Alexis King)

Validate Once at the Boundary, Trust Inside

When the same precondition is checked at every layer — the controller checks the email format, the service checks it again, the model checks it again, the repository checks it again — the precondition is being treated as untrusted everywhere. Validation belongs at the boundary where untyped data enters the system. Inside, the type should say "this is a ValidEmail, you don't need to check." Re-validation is a sign the system has no clear inside.

Incorrect (every layer re-validates the same thing):

// controller:
async function createUserController(req: Request) {
  const { email, name } = req.body;
  if (typeof email !== 'string' || !email.includes('@')) return error('bad email');
  if (typeof name !== 'string' || name.length < 2) return error('bad name');
  return userService.create({ email, name });
}

// service:
async function create(input: { email: string; name: string }) {
  if (!input.email.includes('@')) throw new Error('bad email');     // again
  if (input.name.length < 2) throw new Error('bad name');           // again
  return userRepository.insert(input);
}

// repository:
async function insert(input: { email: string; name: string }) {
  if (!input.email.includes('@')) throw new Error('bad email');     // and again
  return db.users.create({ data: input });
}
// 6 lines of defensive duplication. Update the validation rules → edit 3 places.

Correct (validate at the boundary; the type carries the guarantee inside):

// validation/user.ts — at the boundary:
import { z } from 'zod';

const CreateUserInputSchema = z.object({
  email: z.string().email(),
  name: z.string().min(2),
});
type CreateUserInput = z.infer<typeof CreateUserInputSchema>;

// controller:
async function createUserController(req: Request) {
  const parsed = CreateUserInputSchema.safeParse(req.body);
  if (!parsed.success) return error(parsed.error);
  return userService.create(parsed.data);
}

// service:
async function create(input: CreateUserInput) {           // type guarantees valid shape
  return userRepository.insert(input);
}

// repository:
async function insert(input: CreateUserInput) {            // ditto
  return db.users.create({ data: input });
}
// Validation: 1 place. The type system carries the guarantee the rest of the way.

The mental model — "parse, don't validate":

validate returns boolean: callers must still believe it. parse returns a new type that proves the validation succeeded. Inside the system, code receives parsed types and can trust them. The trust boundary is visible: it's the parser.

Common reflexive double-validations to delete:

• The controller validated id is a UUID; the service validates again. Trust the parsed input.
• The form library produced a typed value; you assert its constraints again before submit.
• The ORM constraints the field; you constrain it again in the application code.
• The previous function returned a NonEmpty<T>; you check .length > 0 anyway.

When NOT to use this pattern:

• The internal function is a public library API that JS consumers can call without types. Defend at its boundary too — that's where untyped data enters.
• The data may have been mutated between boundary and use, and you can't make it immutable. Then a fresh check is warranted at the new use site — though usually the better fix is "make it immutable."
• The cost of being wrong is catastrophic (a financial calculation, a permissions check that gates destructive ops). Belt-and-braces is acceptable. Document why; otherwise it gets ripped out later.

Reference: Alexis King — Parse, Don't Validate

Derive Booleans From the Data, Don't Track Them Separately

Booleans like isEmpty, hasErrors, isComplete, isOverdue, isPaid are almost always questions about other state, not new facts. Storing them creates a maintenance burden: every place that changes the underlying data must remember to recompute the boolean. The fix is to make them functions (or properties) of the data they describe — the question gets asked at read time, and the answer is automatically correct.

Incorrect (tracking the boolean as separate state):

class TodoList {
  items: Todo[] = [];
  isEmpty: boolean = true;
  hasOverdue: boolean = false;
  allCompleted: boolean = false;

  add(todo: Todo) {
    this.items.push(todo);
    this.isEmpty = false;
    this.hasOverdue = this.hasOverdue || todo.dueDate < new Date();
    this.allCompleted = this.items.every(t => t.completed);
  }

  complete(id: string) {
    const t = this.items.find(t => t.id === id);
    if (t) t.completed = true;
    this.allCompleted = this.items.every(t => t.completed);
    // Did we update `hasOverdue` too? No — we forgot. Bug.
  }

  remove(id: string) {
    this.items = this.items.filter(t => t.id !== id);
    this.isEmpty = this.items.length === 0;
    // Also forgot to update hasOverdue and allCompleted. Two more bugs.
  }
}

Correct (the booleans ask their question at read time):

class TodoList {
  items: Todo[] = [];

  get isEmpty()     { return this.items.length === 0; }
  get hasOverdue()  { return this.items.some(t => t.dueDate < new Date()); }
  get allCompleted(){ return this.items.every(t => t.completed); }

  add(todo: Todo)     { this.items.push(todo); }
  complete(id: string){ const t = this.items.find(t => t.id === id); if (t) t.completed = true; }
  remove(id: string)  { this.items = this.items.filter(t => t.id !== id); }
}
// Mutations are trivial. Every boolean is always correct.
// The "is the boolean stale?" failure mode no longer exists.

In React, same idea — no need for a boolean state to mirror data:

// Incorrect:
const [items, setItems] = useState<Todo[]>([]);
const [isEmpty, setIsEmpty] = useState(true);
useEffect(() => { setIsEmpty(items.length === 0); }, [items]);

// Correct:
const [items, setItems] = useState<Todo[]>([]);
const isEmpty = items.length === 0;

Symptoms:

• A boolean whose value is data.length === 0, data.every(...), data.some(...), or data.includes(...).
• Every mutation function in a class updates a "status" boolean.
• A bug ticket "X says empty but the list has items."
• A "refresh" / "recompute" function that updates multiple booleans together.

When NOT to use this pattern:

• The boolean is not a question about other state but an independent user input (e.g. isPinned — set by the user, not derived from data). Keep it.
• Computing the boolean is genuinely expensive and reads happen far more often than writes. Cache it explicitly, but write a single function that returns the current answer and call it from getters and writes alike — don't sprinkle invalidation.

Reference: React docs — Avoid redundant state

Turn Cached Fields Into Getters Until Profiling Proves Otherwise

A surprising amount of code is a stored field that "caches" a cheap computation, with elaborate logic to keep the cache in sync. Most of these cases are premature — the computation is fast enough that the field gains you nothing, but it costs you a permanent risk of "cache out of sync." Start with a getter; promote to a cached field only when profiling shows a real cost.

Incorrect (cached field with manual invalidation):

class Order {
  items: LineItem[] = [];
  private _total: number = 0;
  private _totalDirty: boolean = true;

  addItem(item: LineItem) {
    this.items.push(item);
    this._totalDirty = true;
  }

  removeItem(id: string) {
    this.items = this.items.filter(i => i.id !== id);
    this._totalDirty = true;
  }

  applyDiscount(percent: number) {
    this.items.forEach(i => { i.price *= (1 - percent / 100); });
    this._totalDirty = true;
    // Did we remember to invalidate everywhere we mutate items? No — there's a mutation
    // in `bulkImport()` 200 lines below that forgets to set `_totalDirty = true`. Bug.
  }

  get total(): number {
    if (this._totalDirty) {
      this._total = this.items.reduce((s, i) => s + i.price * i.qty, 0);
      this._totalDirty = false;
    }
    return this._total;
  }
}
// 20 lines of caching machinery for a sum that takes microseconds.

Correct (just compute it):

class Order {
  items: LineItem[] = [];

  get total(): number {
    return this.items.reduce((s, i) => s + i.price * i.qty, 0);
  }

  addItem(item: LineItem)         { this.items.push(item); }
  removeItem(id: string)          { this.items = this.items.filter(i => i.id !== id); }
  applyDiscount(percent: number)  { this.items.forEach(i => { i.price *= (1 - percent / 100); }); }
}
// The "cache invalidation" failure mode is gone because there is no cache.
// If profiling later shows `total` is a hot path with thousands of items, add memoisation.

The promote-to-cache test (apply IN ORDER, stop at first "no"):

1. Profile. Is total (or whichever derivation) measurably expensive in a real scenario? 2. Count calls. Is it called many times per change, or once per change? 3. Check the change-to-read ratio. Caching pays off when reads ≫ writes. 4. Only then add memoisation. And do it with a single well-known pattern (a WeakMap keyed on the source, a memoise helper from a library) — not a custom dirty-flag system.

Symptoms of premature caching:

• A _dirty flag, a lastUpdated field, or "remember to invalidate the cache" comments.
• A method whose only job is to mark caches dirty.
• A bug pattern "the displayed value is stale."
• Tests that assert specific cached-vs-fresh behaviour.

When NOT to use this pattern:

• The derivation is genuinely expensive (a network call, a database query, a multi-second computation) — caching is the right answer; just use a proven pattern, not hand-rolled flags.
• The derivation produces a value used as a key in some collection where identity matters — then you need stable references and a careful cache.

Reference: Donald Knuth — Structured Programming with go to Statements (§1) (the "premature optimization" essay)

Compute What You Can Compute; Store Only What You Can't

Every stored value is a promise to keep it in sync with every input that feeds it. When fullName = firstName + ' ' + lastName is held in its own variable, every place that updates firstName must also update fullName — or the bug is "the name doesn't refresh." The mental-model gap is that the engineer thinks of fullName as a thing, when it's really a view of other things. Computed values don't need storing; they need a getter, a memo, or just an inline expression.

Incorrect (React component with redundant state for derived values):

function CartSummary({ items }: { items: CartItem[] }) {
  const [itemCount, setItemCount] = useState(0);
  const [subtotal, setSubtotal] = useState(0);
  const [hasItems, setHasItems] = useState(false);

  useEffect(() => {
    setItemCount(items.length);
    setSubtotal(items.reduce((s, it) => s + it.price * it.qty, 0));
    setHasItems(items.length > 0);
  }, [items]);
  // Three pieces of state and one effect — for values that are pure functions of `items`.
  // Every render now has stale-state-during-update risk. Bugs lurk in the gap between
  // when `items` changes and when the effect catches up.

  return <Footer count={itemCount} subtotal={subtotal} empty={!hasItems} />;
}

Correct (no state at all — the values are just computed):

function CartSummary({ items }: { items: CartItem[] }) {
  const itemCount = items.length;
  const subtotal  = items.reduce((s, it) => s + it.price * it.qty, 0);
  const hasItems  = itemCount > 0;
  return <Footer count={itemCount} subtotal={subtotal} empty={!hasItems} />;
  // No useState. No useEffect. No stale-state window. Always correct on every render.
  // If `subtotal` is expensive, wrap it in useMemo. Don't promote it to state.
}

Outside React — getters instead of fields:

// Incorrect:
class Invoice {
  items: LineItem[];
  total: number;  // updated by every method that mutates items
  itemCount: number;
  isEmpty: boolean;

  addItem(item: LineItem) {
    this.items.push(item);
    this.total += item.price * item.qty;
    this.itemCount++;
    this.isEmpty = false;
  }
  // Three writes per mutation. One missed write = inconsistent invoice.
}

// Correct:
class Invoice {
  items: LineItem[];

  get total()     { return this.items.reduce((s, it) => s + it.price * it.qty, 0); }
  get itemCount() { return this.items.length; }
  get isEmpty()   { return this.items.length === 0; }

  addItem(item: LineItem) { this.items.push(item); }
  // One source of truth. Derived values can never desync.
}

Symptoms of "stored what could be computed":

• A useEffect (or componentDidUpdate) whose only job is to copy one piece of state into another.
• A field on a class whose value is set every time another field changes.
• A "refresh" function that walks a list of state and updates each entry.
• A bug ticket of the form "X doesn't update when Y changes."
• Two pieces of state that are always a function of each other.

When NOT to use this pattern:

• The derivation is genuinely expensive and runs on every render — use useMemo/getter with memoisation, not state.
• The "derived" value is actually an independent input that the user can override — then it's not derived; it's its own state.
• You need to capture the value at a specific moment (a snapshot) — that's state, not derivation. Example: [priceAtPurchase, setPriceAtPurchase] records what the user agreed to pay even after the menu price changes. The current price would re-derive; the agreed price needs to be stored.

Reference: React docs — You Might Not Need an Effect

Pick One Source of Truth; Derive the Rest

When two pieces of state must always agree (a selected item id and the selected item object, a list and its sorted version, a Date and its formatted string), one of them is the truth and the other is a view. Storing both means every write site must update both, every read site must trust both, and bugs of the form "the IDs disagree" become a permanent risk. Pick the smallest, most stable representation as the truth, and compute the others where they're needed.

Incorrect (two pieces of state for one fact):

function ProductSelector({ products }: { products: Product[] }) {
  const [selectedId, setSelectedId]         = useState<string | null>(null);
  const [selectedProduct, setSelectedProduct] = useState<Product | null>(null);

  const handleSelect = (id: string) => {
    setSelectedId(id);
    setSelectedProduct(products.find(p => p.id === id) ?? null);
    // Two setters. If you forget one (or get the order wrong, or products refetches
    // in between), they disagree. Bugs of the form "the id says X but the object is Y".
  };

  return <Details product={selectedProduct} />;
}

Correct (one piece of state; the other is derived where used):

function ProductSelector({ products }: { products: Product[] }) {
  const [selectedId, setSelectedId] = useState<string | null>(null);
  const selectedProduct = selectedId ? products.find(p => p.id === selectedId) : null;

  return <Details product={selectedProduct} />;
  // selectedId is the truth. selectedProduct is computed on render.
  // When `products` refetches and changes shape, the derivation re-runs automatically.
}

Choosing which side is the truth:

The other one is then a function of it.

Other cases of two-truths trouble:

• A searchQuery state and a filteredResults state — keep the query; derive the results in render or with a memoised function call.
• A list of items and a selectedItemsSet — pick one and derive the other (selectedIds is usually the stable choice; the array of selected objects is derived).
• A user object and a userId — almost always the id is the truth; the user is the derivation (refetched when stale).

Symptoms of two-truth state:

• Two useState calls that are always updated together in every handler.
• A useEffect that copies one piece of state to another.
• A bug pattern "X and Y disagree" or "X is stale relative to Y."
• Tests that assert both pieces in lockstep.

When NOT to use this pattern:

• The two pieces represent genuinely independent facts that coincidentally match in the simple case — keep them separate.
• The derivation is asynchronous (fetching the object given the id) — then the derivation is its own loading state, not free derivation. Use a query hook for that.

Reference: React docs — Choosing the State Structure

Let the URL or Route Be the State, Not a Mirror of It

For things the URL already represents — current tab, search query, page number, selected entity id — the URL is the state, and the engineer's job is to read it. Mirroring it into local component state creates two truths: now any change must update both, and any external source (a deep link, a back button, a refresh) finds them out of sync. The judgment skill is recognising that browser routing already solves "what is the user looking at?" and not building a parallel system.

Incorrect (URL and local state both track the same fact):

function SearchPage() {
  const [searchParams, setSearchParams] = useSearchParams();
  const [query, setQuery] = useState(searchParams.get('q') ?? '');
  const [page, setPage]   = useState(Number(searchParams.get('page') ?? '1'));

  // Keep local state in sync with URL:
  useEffect(() => { setQuery(searchParams.get('q') ?? ''); }, [searchParams]);
  useEffect(() => { setPage(Number(searchParams.get('page') ?? '1')); }, [searchParams]);

  const handleSearch = (q: string) => {
    setQuery(q);                                          // local
    setSearchParams({ q, page: '1' });                    // URL
    setPage(1);                                           // local again
    // Three updates for one user action. If you forget one, the bug is "page resets on type"
    // or "URL doesn't reflect query." Both have happened to everyone who wrote this code.
  };
  // ...
}

Correct (the URL is the state; derive everything else from it):

function SearchPage() {
  const [searchParams, setSearchParams] = useSearchParams();
  const query = searchParams.get('q') ?? '';
  const page  = Number(searchParams.get('page') ?? '1');

  const handleSearch = (q: string) => {
    setSearchParams({ q, page: '1' });
  };
  // One source of truth. Refresh, back, deep link — all work without extra wiring.
}

The same idea applies beyond URLs:

• Form library state. If react-hook-form (or your form lib) already tracks the value, don't copy it into local state.
• Query cache. If TanStack Query / SWR holds the data, don't copy it into a const [data] mirror.
• Server cookies / sessions. If the server tells you who the user is, don't also track it client-side and reconcile.
• Route params. id from useParams() is the state — don't useState a copy.

Symptoms:

• An effect that copies router state into local state.
• A handler that calls two setters: one for URL, one for local.
• Tests for "URL and local state agree" assertions.
• Bug pattern: "back button takes me to the right URL but the page shows the previous query."

When NOT to use this pattern:

• You need a debounced staging state before committing to the URL (don't push to URL on every keystroke). Then the local state has a real role: it holds the in-progress value. The committed state is still the URL.
• The URL is shared and you don't want every transient UI change to appear in browser history — use replace, not push, but still keep the URL as the truth.

Reference: TkDodo — Don't over useState, Nuqs docs

Replace Many Hardcoded Copies With One Table

When the same shape — a feature flag with its default, a route with its handler, a column with its formatter — appears N times across the code with only the names and values changing, the right form is a single declarative table. Hardcoded copies invite drift (one entry forgotten when a global rule changes), and they hide the set of valid entries behind grep results. A table makes the set first-class.

Incorrect (N copies of the same shape, one per entity):

// In featureFlags.ts:
export function isCheckoutV2Enabled(user: User): boolean {
  if (!isFeatureFlagsLoaded()) return false;
  const flag = remoteConfig.get('checkout_v2');
  if (!flag) return false;
  if (flag.disabled) return false;
  if (flag.allowlist && !flag.allowlist.includes(user.id)) return false;
  return true;
}

export function isNewDashboardEnabled(user: User): boolean {
  if (!isFeatureFlagsLoaded()) return false;
  const flag = remoteConfig.get('new_dashboard');
  if (!flag) return false;
  if (flag.disabled) return false;
  if (flag.allowlist && !flag.allowlist.includes(user.id)) return false;
  return true;
}

export function isBetaPricingEnabled(user: User): boolean { /* same shape again */ }
export function isFastSearchEnabled(user: User): boolean { /* same shape again */ }
// 6+ identical functions. Add a "geo restriction" to flags → edit every function.

Correct (the flag set is data; the logic runs once):

type FlagKey = 'checkout_v2' | 'new_dashboard' | 'beta_pricing' | 'fast_search';

export function isEnabled(key: FlagKey, user: User): boolean {
  if (!isFeatureFlagsLoaded()) return false;
  const flag = remoteConfig.get(key);
  if (!flag) return false;
  if (flag.disabled) return false;
  if (flag.allowlist && !flag.allowlist.includes(user.id)) return false;
  return true;
}
// Adding a flag = adding a string to `FlagKey`.
// Adding a rule (geo restriction) = editing one function. Once.

Common shapes that often live as copies but want to be tables:

• HTTP routes + handlers (app.get('/a', handlerA); app.get('/b', handlerB)) → array of {path, handler} rows.
• Permissions per role (canRead = role === 'admin' || role === 'editor'; canWrite = role === 'admin') → matrix table.
• Column definitions for a grid (one component per column with formatter, sort, filter inline) → array of column-descriptor rows.
• Validation rules per field, defaults per environment, error messages per error code.

Symptoms:

• Three or more functions/objects with the same shape and one varying token (usually a string key).
• Comments like // TODO: when adding a flag here, also update X, Y, Z.
• A grep that returns N matches for a pattern, all near-identical.
• Tests parameterised over the same N entities, asserting the same property of each.

When NOT to use this pattern:

• The entries differ in behaviour, not just data — keeping them as functions lets each one diverge naturally. Force into a table only when the shape is genuinely stable.

Reference: Tidy First? — Replace Conditional With Map (Kent Beck)

Collapse Identical DTOs, DB Rows, and Domain Objects

The "clean architecture" instinct produces three types per entity: a database row type, a domain type, and a DTO for the API — plus the mappers between them. When the three types are the same shape with the same field names and the same semantics, the layers are decorative. Each new field requires editing three types and two mappers, and tests exist mainly to verify the mappers don't drop fields. Either keep three types because the shapes genuinely differ, or use one type and accept that the layers are conceptual, not physical.

Incorrect (three identical types and two identity mappers per layer):

// db/types.ts
export type UserRow = { id: string; email: string; name: string; createdAt: Date };

// domain/types.ts
export type User = { id: string; email: string; name: string; createdAt: Date };

// api/types.ts
export type UserDto = { id: string; email: string; name: string; createdAt: Date };

// db/mappers.ts
export const rowToUser = (r: UserRow): User =>
  ({ id: r.id, email: r.email, name: r.name, createdAt: r.createdAt });

// api/mappers.ts
export const userToDto = (u: User): UserDto =>
  ({ id: u.id, email: u.email, name: u.name, createdAt: u.createdAt });

// Adding a field → 5 places to edit. Tests verify identity functions. No information added.

Correct (option A — collapse to one type when shapes are truly identical):

export type User = { id: string; email: string; name: string; createdAt: Date };
// Used by the DB driver, the domain logic, and the API.
// If a layer needs to add a field, it does — and the layered separation re-emerges naturally.

Correct (option B — keep separate types only when they really differ):

export type UserRow = { id: string; email: string; name: string; created_at: string };
// DB uses snake_case + string timestamps because the driver returns them that way.

export type User = { id: string; email: string; name: string; createdAt: Date };
// Domain uses camelCase + Date.

export type UserDto = Omit<User, 'email'>;
// API hides email from external consumers.

// Mappers now have a real job: type-system-distinct names, format conversions, field hiding.

Symptoms of decorative layers:

• The "mapper" is (x) => ({ ...x }) or assigns identically named fields.
• Adding a field to the DB requires PRs across three files just to surface it.
• "Domain type" has no methods, no invariants, no behaviour — it's just a struct.
• The DTO is the domain object minus zero fields and plus zero fields.

When NOT to use this pattern:

• The layers are different today, even if barely — e.g. the DB stores email_normalized and the domain uses email. Keep them separate; the separation has paid a cost.
• You expect divergence soon for a known reason (API versioning, schema migration in flight). Time-bound the duplication.
• The team's architectural standard requires the layering and the cost is accepted as documentation/discipline. Then the question becomes: what fields are different? Make those visible.

Reference: A Philosophy of Software Design — Pass-Through Methods (John Ousterhout)

Lift Shared Lines Out of Mirrored Branches

When the if and else branches of a conditional contain the same five lines plus one different line, the common lines are not about the condition — they always run. The condition is only deciding the differing line. Pulling shared lines out (above or below the conditional) shrinks the function, makes the actual decision visible, and prevents the common case where someone edits one branch and forgets the other.

Incorrect (mirrored branches with one real difference):

function recordPayment(payment: Payment, user: User): Receipt {
  if (payment.method === 'card') {
    const receipt = createReceipt(payment);
    receipt.lineItems = payment.lineItems;
    receipt.timestamp = Date.now();
    receipt.processor = 'stripe';
    saveReceipt(receipt);
    notifyAccounting(receipt);
    return receipt;
  } else {
    const receipt = createReceipt(payment);
    receipt.lineItems = payment.lineItems;
    receipt.timestamp = Date.now();
    receipt.processor = 'ach';
    saveReceipt(receipt);
    notifyAccounting(receipt);
    return receipt;
  }
  // 12 lines, of which 10 are duplicated. The condition decides ONE field.
}

Correct (the shared work happens once; the decision is one line):

function recordPayment(payment: Payment, user: User): Receipt {
  const receipt = createReceipt(payment);
  receipt.lineItems = payment.lineItems;
  receipt.timestamp = Date.now();
  receipt.processor = payment.method === 'card' ? 'stripe' : 'ach';
  saveReceipt(receipt);
  notifyAccounting(receipt);
  return receipt;
}
// The condition is now where the decision is, not where the duplication lives.

A more interesting case — the diff is in the middle:

// Incorrect:
if (kind === 'export') {
  validate(payload);
  authorize(user, 'export');
  log('export.start');
  doExport(payload);
  log('export.done');
} else {
  validate(payload);
  authorize(user, 'import');
  log('import.start');
  doImport(payload);
  log('import.done');
}

// Correct (lift the structure, parameterize the inner action and labels):
const action  = kind === 'export' ? doExport : doImport;
const verb    = kind;
validate(payload);
authorize(user, verb);
log(`${verb}.start`);
action(payload);
log(`${verb}.done`);
// Five lines, no branch — the difference is captured in two variables.

Symptoms:

• Two branches with the same length and almost-identical structure.
• A code review comment of the form "you forgot to update the else branch."
• Adding a feature requires editing both branches in symmetric ways.
• The diff between branches highlights as a single line (or a few).

When NOT to use this pattern:

• The branches happen to look similar but model genuinely different operations that may diverge — refactor only after the duplication has appeared three times.
• The "shared" parts have subtly different orderings or interleavings — lifting them changes behaviour. Read carefully before lifting.

Reference: Refactoring — Consolidate Duplicate Conditional Fragments (Martin Fowler)

Parameterize Two Functions That Differ by a Literal

When you see two functions named almost the same — sendEmailToCustomer / sendEmailToVendor, getActiveUsers / getInactiveUsers, formatUSD / formatEUR — and their bodies are character-for-character identical except a single literal, you have one function masquerading as two. Each twin must be kept in sync forever, and they always drift. The judgment skill is identifying the axis that varies (a recipient role, a flag, a currency code) and lifting it out.

Incorrect (two functions, one diff each):

async function notifyCustomerOfPriceChange(customerId: string): Promise<void> {
  const customer = await db.customers.find(customerId);
  await mailer.send({
    to: customer.email,
    template: 'price-change',
    subject: 'Important: Your price has changed',
    cc: 'customer-success@acme.com',
  });
  await audit.log('price_change_notified', { recipientId: customerId, role: 'customer' });
}

async function notifyVendorOfPriceChange(vendorId: string): Promise<void> {
  const vendor = await db.vendors.find(vendorId);
  await mailer.send({
    to: vendor.email,
    template: 'price-change',
    subject: 'Important: Your price has changed',
    cc: 'vendor-relations@acme.com',
  });
  await audit.log('price_change_notified', { recipientId: vendorId, role: 'vendor' });
}
// One axis varies (role: customer vs vendor → different table, different cc).
// Everything else is duplicated. Add a header to the email and you edit both.

Correct (lift the varying axis to a parameter):

type Role = 'customer' | 'vendor';

const CONFIG: Record<Role, { table: 'customers' | 'vendors'; cc: string }> = {
  customer: { table: 'customers', cc: 'customer-success@acme.com' },
  vendor:   { table: 'vendors',   cc: 'vendor-relations@acme.com' },
};

async function notifyOfPriceChange(role: Role, id: string): Promise<void> {
  const { table, cc } = CONFIG[role];
  const recipient = await db[table].find(id);
  await mailer.send({
    to: recipient.email,
    template: 'price-change',
    subject: 'Important: Your price has changed',
    cc,
  });
  await audit.log('price_change_notified', { recipientId: id, role });
}
// One function. Adding a "partner" role is one new line in CONFIG.

Distinguishing real differences from cosmetic ones:

Look at the body line by line. If every difference between the two functions is:

• A literal value (string, number, currency code), or
• A different table/collection name, or
• A different audit category

...then the function is one function. If the differences include different control flow, different error handling, different validation rules, then they may genuinely be two functions — and you should split that distinction into shared and divergent parts, not collapse them.

When NOT to use this pattern:

• Two functions that look similar today but are being held apart deliberately because they evolve independently (e.g. compliance rules for one role about to change). Premature unification re-couples them. Note this with a comment if so.

Reference: Refactoring — Parameterize Function (Martin Fowler)

Collapse Parallel Types That Share a Shape

When User, Customer, and Contact all carry {id, name, email, phone} and the difference is only "where in the system they live," you have one shape with three labels, not three things. The duplication is invisible to linters because the names are different, but every consumer ends up writing the same logic three times and every change happens in triplicate. Either unify them, or — when they're genuinely distinct — make the distinction the only thing that differs.

Incorrect (three types that pretend to be different):

type User     = { id: string; name: string; email: string; phone: string; createdAt: Date };
type Customer = { id: string; name: string; email: string; phone: string; createdAt: Date };
type Contact  = { id: string; name: string; email: string; phone: string; createdAt: Date };

function userToCustomer(u: User): Customer {
  return { id: u.id, name: u.name, email: u.email, phone: u.phone, createdAt: u.createdAt };
}
function customerToContact(c: Customer): Contact {
  return { id: c.id, name: c.name, email: c.email, phone: c.phone, createdAt: c.createdAt };
}
// Three identity functions. Three places to keep in sync. Zero meaningful distinctions.

Correct (option A — one type, no distinction to preserve):

type Person = { id: string; name: string; email: string; phone: string; createdAt: Date };
// Used everywhere User/Customer/Contact used to be. Mappers gone.

Correct (option B — the distinction is real; make it the only difference):

type PersonBase = { id: string; name: string; email: string; phone: string; createdAt: Date };

type User     = PersonBase & { kind: 'user';     lastLoginAt: Date };
type Customer = PersonBase & { kind: 'customer'; lifetimeValue: number };
type Contact  = PersonBase & { kind: 'contact';  source: string };
// Now the types document what's actually different.
// Logic that doesn't care about the difference can take `PersonBase`.
// Logic that does care discriminates on `kind`.

Symptoms of parallel-types duplication:

• Two or more types with identical fields and no behavioural difference at use sites.
• A folder of "mapper" or "DTO converter" functions that are essentially x => x.
• Tests that mostly verify the mappers preserve fields.
• The fields are renamed in some types (emailAddress vs email) but mean the same thing — that's the same problem one indirection deeper.

When NOT to use this pattern:

• The types come from distinct external systems with their own naming conventions you don't control (a DB schema, a SOAP API). Keep them separate at the boundary, but map to one internal type once — not at every consumer.
• The types share fields today but are expected to diverge along well-known axes. Premature unification can hurt; lock them in only when both directions are stable.

Reference: Domain Modeling Made Functional — chap. 6 (Scott Wlaschin)

Compose Shared Fields Instead of Inheriting From a Base Class

Inheritance is for substitutability — Dog is-a Animal, you can pass either where Animal is expected. When inheritance is used to "share fields and helper methods," you've taken a small data-sharing problem and bound your types into a rigid hierarchy. Every subclass must accept every base-class field and every base-class method, forever. Composition (an interface, a field of a shared type, a mixin function) does the same job without the lock-in.

Incorrect (`BaseEntity` glued to every model so they can share `id`/`createdAt`):

abstract class BaseEntity {
  id: string;
  createdAt: Date;
  updatedAt: Date;
  protected log(message: string) { console.log(`[${this.id}] ${message}`); }
  abstract validate(): void;
}

class User extends BaseEntity {
  email: string;
  validate() { if (!this.email.includes('@')) throw new Error('bad email'); }
}

class Order extends BaseEntity {
  total: number;
  validate() { if (this.total < 0) throw new Error('negative'); }
}

// Three problems:
// 1. Order needs `updatedAt` because Base says so, even if Order is immutable.
// 2. `validate()` lives on Base but has nothing structural in common across types.
// 3. Adding a new mixin (say, `Versioned`) requires a new base or multiple-inheritance gymnastics.

Correct (compose the shared shape; functions handle the shared verbs):

type Identified = { id: string; createdAt: Date };

type User  = Identified & { kind: 'user';  email: string };
type Order = Identified & { kind: 'order'; total: number };

const validateUser  = (u: User)  => { if (!u.email.includes('@')) throw new Error('bad email'); };
const validateOrder = (o: Order) => { if (o.total < 0)            throw new Error('negative'); };

const logFor = (e: Identified) => (msg: string) => console.log(`[${e.id}] ${msg}`);
// Each type carries exactly the fields it needs.
// `validate` is N independent functions, not one virtual hook.
// Adding `Versioned` is intersecting another type — no hierarchy to refactor.

Symptoms of inheritance-for-sharing:

• The base class has fields used by every subclass for different reasons.
• Subclasses override a base method with super.foo() plus a tweak (the "fragile base class" problem).
• The hierarchy is two levels deep "because we needed to share with siblings."
• The base class has both abstract methods and concrete helpers — the abstract part is the real polymorphism; the helpers are field-sharing wearing a hood.

When NOT to use this pattern:

• Inheritance models a genuine "is-a substitutable" relationship — class CreditCardPayment extends Payment where every Payment consumer treats them uniformly. Keep it.
• Framework requirements force a base class (class extends React.Component, class extends NSObject). The framework owns the hierarchy; you don't get a choice.

Reference: Design Patterns — "Favor object composition over class inheritance" (Gang of Four)

Model the Problem as Data Before Writing Procedure

When a function grows long because it has many cases, each requiring its own branch, the cases are usually data, not code. The cure is to write down the cases as a table, list, or graph, and then write one small piece of code that interprets that data. The result is shorter, easier to extend (new case = new row), easier to test (parameterise on rows), and easier to read (one mechanism, many configs).

Incorrect (each shipping method is a branch in one growing procedure):

function calculateShipping(method: string, weight: number, country: string): number {
  if (method === 'standard' && country === 'US') {
    return weight < 1 ? 4.99 : weight < 5 ? 9.99 : 19.99;
  }
  if (method === 'standard' && country === 'EU') {
    return weight < 1 ? 6.99 : weight < 5 ? 14.99 : 29.99;
  }
  if (method === 'express' && country === 'US') {
    return weight < 1 ? 14.99 : weight < 5 ? 24.99 : 49.99;
  }
  if (method === 'express' && country === 'EU') {
    return weight < 1 ? 19.99 : weight < 5 ? 34.99 : 59.99;
  }
  if (method === 'overnight' && country === 'US') {
    return weight < 1 ? 29.99 : weight < 5 ? 49.99 : 99.99;
  }
  // ...continues for every method × country × weight tier. Add a country: edit every branch.
  throw new Error('Unsupported');
}

Correct (the cases are data; the procedure is one tiny interpreter):

type Tier = { maxWeight: number; price: number };
type Region = { country: string; method: string; tiers: Tier[] };

const RATES: Region[] = [
  { country: 'US', method: 'standard',  tiers: [{ maxWeight: 1, price: 4.99 },  { maxWeight: 5, price: 9.99 },  { maxWeight: Infinity, price: 19.99 }] },
  { country: 'EU', method: 'standard',  tiers: [{ maxWeight: 1, price: 6.99 },  { maxWeight: 5, price: 14.99 }, { maxWeight: Infinity, price: 29.99 }] },
  { country: 'US', method: 'express',   tiers: [{ maxWeight: 1, price: 14.99 }, { maxWeight: 5, price: 24.99 }, { maxWeight: Infinity, price: 49.99 }] },
  { country: 'EU', method: 'express',   tiers: [{ maxWeight: 1, price: 19.99 }, { maxWeight: 5, price: 34.99 }, { maxWeight: Infinity, price: 59.99 }] },
  { country: 'US', method: 'overnight', tiers: [{ maxWeight: 1, price: 29.99 }, { maxWeight: 5, price: 49.99 }, { maxWeight: Infinity, price: 99.99 }] },
];

function calculateShipping(method: string, weight: number, country: string): number {
  const region = RATES.find(r => r.country === country && r.method === method);
  if (!region) throw new Error('Unsupported');
  return region.tiers.find(t => weight < t.maxWeight)!.price;
}
// New region → one new row. New tier rule → change the interpreter once.
// The procedure is 3 lines. The data carries the variability.

Cues that procedure should be data:

• The function is long, but every branch has the same shape (lookup → return).
• Adding a new case means copy-pasting an existing branch and changing literals.
• Tests for the function are mostly checking that each branch returns the right constant.
• You can describe the function's behaviour as "for each X, do Y" — that's a table.

When NOT to use this pattern:

• The cases truly differ in behaviour, not just values — e.g. one branch calls a different external service, another writes to a different table. Then they're real cases, not table rows. (Though even then, a discriminated union may be cleaner than a long if-chain.)
• There are only two or three cases and they're never going to grow. Don't over-engineer for hypothetical extension.

Reference: Tidy First? — Replace Conditional With Map (Kent Beck)

Use a Function When the Class Has No Identity

A class earns its existence when it holds state across method calls, or when polymorphism is genuinely needed. A class with no fields (or only fields set once at construction and never mutated) where every method is a pure transform of inputs is a function. Wrapping it in a class adds construction sites, dependency injection, mocking ceremony, and forces every caller to know who instantiates it — for nothing.

Incorrect (a class doing what a function does, with full DI ceremony):

export class PriceFormatter {
  constructor(private readonly locale: string) {}

  format(amount: number, currency: string): string {
    return new Intl.NumberFormat(this.locale, { style: 'currency', currency }).format(amount);
  }
}

// At every call site:
const formatter = new PriceFormatter(user.locale);
const text = formatter.format(99.99, 'EUR');

// And the DI module that wires it, the test that mocks it, the interface that types it...
// All of that ceremony for what is a single Intl call.

Correct (a function — no construction, no DI, no mock):

export function formatPrice(amount: number, currency: string, locale: string): string {
  return new Intl.NumberFormat(locale, { style: 'currency', currency }).format(amount);
}

// Call site:
const text = formatPrice(99.99, 'EUR', user.locale);

Symptoms that the class wants to be a function:

• Constructor stores values that are only used to thread into one method's call.
• Every method is static or could be.
• Tests do new X(...) once and immediately call one method.
• The class has no internal state that mutates across calls.
• The class's interface has exactly one method (the "command object" smell).

When NOT to use this pattern:

• The class holds genuine state — a connection pool, a cache, a counter. Keep it.
• You need polymorphism — multiple implementations swappable behind an interface. Keep it.
• The constructor performs expensive setup that should be reused (compile a regex, open a DB connection). Keep it, but consider a module-level singleton instead.

Stateful-looking but actually function (partial application):

// If you do want to "bind" the locale once for several formatters:
export const makePriceFormatter = (locale: string) =>
  (amount: number, currency: string) =>
    new Intl.NumberFormat(locale, { style: 'currency', currency }).format(amount);

const fmt = makePriceFormatter(user.locale);
fmt(99.99, 'EUR');
// A closure is a one-line stateful object. No class needed.

Reference: A Philosophy of Software Design — John Ousterhout

Rename Manager/Helper/Util Classes Until the Real Verb Appears

UserManager, OrderHelper, StringUtils, DataProcessor — these names exist because the engineer couldn't find a coherent concept and reached for a noun-shaped bag. A manager class is usually a grab-bag of unrelated verbs glued together by accident of subject. The fix isn't to rename it; it's to ask, for each method: "what does this actually do?" Each verb usually points to a different real concept — a domain function, a different module, or a method on the data itself.

Incorrect (one class as a dumping ground for "things involving users"):

class UserManager {
  // Authentication
  async login(email: string, password: string): Promise<Session> { /* ... */ }
  async logout(sessionId: string): Promise<void> { /* ... */ }

  // CRUD
  async createUser(input: CreateUserInput): Promise<User> { /* ... */ }
  async updateUser(id: string, input: UpdateInput): Promise<User> { /* ... */ }

  // Notifications
  async sendWelcomeEmail(user: User): Promise<void> { /* ... */ }
  async sendPasswordReset(user: User): Promise<void> { /* ... */ }

  // Permissions
  canAccess(user: User, resource: Resource): boolean { /* ... */ }

  // Formatting
  displayName(user: User): string { /* ... */ }
}
// 8 methods, 4 unrelated concerns. Every test instantiates the whole thing.
// Every change to one concern reads as a change to "the user manager."

Correct (each verb finds its real home):

// auth/session.ts
export async function login(email: string, password: string): Promise<Session> { /* ... */ }
export async function logout(sessionId: string): Promise<void> { /* ... */ }

// users/repository.ts
export async function createUser(input: CreateUserInput): Promise<User> { /* ... */ }
export async function updateUser(id: string, input: UpdateInput): Promise<User> { /* ... */ }

// notifications/email.ts
export async function sendWelcomeEmail(user: User): Promise<void> { /* ... */ }
export async function sendPasswordReset(user: User): Promise<void> { /* ... */ }

// authorization/policy.ts
export function canAccess(user: User, resource: Resource): boolean { /* ... */ }

// users/model.ts  (or a getter on the User type)
export const displayName = (user: User): string => /* ... */;

The renaming exercise:

For each method on a Manager/Helper/Util class, ask: 1. What does this verb actually do? (sendWelcomeEmail — sends an email) 2. What is its real subject? (Not "the user" — the email system) 3. Would a stranger looking for this code know to look on the Manager? (Usually not — they'd search for the verb.)

If three methods on UserManager have three different real subjects, the class is hiding the real architecture. Splitting it makes the boundaries visible — and usually shrinks total code volume because the over-broad class had shared private helpers it didn't need.

When NOT to use this pattern:

• A Manager/Coordinator that genuinely coordinates across concerns and holds the orchestration state (e.g. a saga, a transaction coordinator). That's a real role. Keep it, but be honest about it.

Reference: Clean Code — Meaningful Names (the "noise word" critique applies directly to Manager/Helper/Data/Info/Util)

Split a God-Function Along Its Cohesive Axis, Not by Line Count

A long function or class is not bad because it's long — it's bad because it bundles unrelated changes. The judgment skill is finding the axis of cohesion: the natural seams along which the work breaks into pieces that change for different reasons. Splitting by line count ("extract every 30 lines into a helper") makes things worse — you get small functions with long names that pass 15 variables around. Splitting by cohesion makes each piece smaller, named after what it owns, and independently changeable.

Incorrect (a "ProcessOrder" that mixes auth, pricing, fulfillment, and notification):

async function processOrder(req: Request, db: DB): Promise<Response> {
  // Auth
  const session = await db.sessions.find(req.cookies.sid);
  if (!session) return { status: 401 };
  const user = await db.users.find(session.userId);
  if (user.disabled) return { status: 403 };

  // Validate cart
  const cart = req.body.cart;
  if (!cart.items?.length) return { status: 400, error: 'empty' };
  for (const item of cart.items) {
    const product = await db.products.find(item.id);
    if (!product || product.stock < item.qty) return { status: 400, error: 'oos' };
  }

  // Price + tax
  let subtotal = 0;
  for (const item of cart.items) {
    const product = await db.products.find(item.id);    // re-fetched — also a bug
    subtotal += product.price * item.qty;
  }
  const tax = user.country === 'US' ? subtotal * 0.08 : subtotal * 0.20;
  const total = subtotal + tax;

  // Charge
  const charge = await stripe.charge(user.cardId, total);
  if (!charge.success) return { status: 402 };

  // Fulfill
  for (const item of cart.items) {
    await db.products.decrement(item.id, item.qty);
  }
  const order = await db.orders.create({ userId: user.id, items: cart.items, total });

  // Notify
  await email.send(user.email, `Order #${order.id} confirmed`);
  await analytics.track('order_placed', { userId: user.id, total });

  return { status: 200, orderId: order.id };
  // 30+ lines mixing 5 unrelated concerns. Test it: every test needs the whole stack.
}

Correct (split by what changes together — each piece has one reason to change):

async function processOrder(req: Request, ctx: Context): Promise<Response> {
  const user = await authenticate(req, ctx);                if (!user) return UNAUTHORIZED;
  const cart = await validateCart(req.body.cart, ctx);      if ('error' in cart) return cart.error;
  const total = priceCart(cart, user.country);
  const charge = await ctx.payments.charge(user.cardId, total);
  if (!charge.success) return PAYMENT_FAILED;
  const order = await fulfill(user, cart, total, ctx);
  await notify(user, order, ctx);
  return ok(order);
  // Each helper has ONE axis of change.
  // priceCart: changes when tax rules change. Testable with a synthetic cart, no DB.
  // fulfill: changes when stock semantics change. Testable in isolation.
  // notify: changes when comm channels change. Easily mocked at one line.
}

Finding the axis:

For each block in the long function, ask: "what causes this block to change?" Blocks that share an answer belong together; blocks with different answers belong apart.

• "When tax rules change" → pricing block
• "When auth rules change" → auth block
• "When fulfillment partner changes" → fulfill block
• "When marketing wants a new email" → notify block

Four different answers means four functions. Same answer means one function with two sub-steps.

When NOT to use this pattern:

• The function is long but every block changes together for the same reason — keep it whole. Length alone is not the enemy; bundled change is.
• Premature splitting introduces parameter-passing chains worse than the original. If you're threading 8 variables to a helper, the helper isn't a clean piece.

Reference: A Philosophy of Software Design — Deep Modules, and On the Criteria To Be Used in Decomposing Systems into Modules (Parnas)

Use the Declarative Form When the Framework Provides One

When a framework offers a declarative form for what you want — JSX in React, a template in Vue/Svelte, a query builder in an ORM, a configuration block in IaC — using document.createElement, string concatenation, or a procedural builder is a step backwards. The declarative form was the framework's whole contribution. Procedural rebuilds inside a declarative system give up the framework's diffing, validation, and tooling — and trade them for lines you maintain by hand.

Incorrect (building DOM imperatively inside a React component):

function UserCard({ user }: { user: User }) {
  const ref = useRef<HTMLDivElement>(null);

  useEffect(() => {
    const root = ref.current!;
    root.innerHTML = '';

    const card = document.createElement('div');
    card.className = 'user-card';

    const name = document.createElement('h3');
    name.textContent = user.name;
    card.appendChild(name);

    const email = document.createElement('a');
    email.href = `mailto:${user.email}`;
    email.textContent = user.email;
    card.appendChild(email);

    root.appendChild(card);
  }, [user]);

  return <div ref={ref} />;
  // Built imperative DOM inside React. React's reconciler is gone.
  // CSP, accessibility, server rendering, hydration — all broken.
  // 15 lines for what is six JSX lines.
}

Correct (let React do its job):

function UserCard({ user }: { user: User }) {
  return (
    <div className="user-card">
      <h3>{user.name}</h3>
      <a href={`mailto:${user.email}`}>{user.email}</a>
    </div>
  );
}
// The framework handles diffing, SSR, and accessibility. Your code is the structure.

Same family across stacks:

• SQL via concatenated strings. Use a query builder or parameterised template literal — db.query's sql\SELECT * FROM users WHERE id = ${id}\`` form. Concatenation is also a SQL injection risk.
• HTML emails via string concat. Use a template (MJML, JSX-email, Handlebars) — concatenation breaks the moment you need encoding or i18n.
• Terraform via `local-exec` shell scripts when a resource exists for the thing you want. The resource has lifecycle, plan, and rollback. The shell has none.
• Webpack/Vite config edited by JS code at build time when a config object would do.
• GraphQL queries built by string concat from arguments. Use a query document with variables; clients cache by document.

Symptoms:

• A useEffect/onMounted that mutates the DOM the framework just rendered.
• String-builder code that ends in .join('') and is passed to innerHTML/dangerouslySetInnerHTML.
• An ORM raw query when the ORM has a method for what you're doing.
• A forEach inside a render function that pushes JSX nodes instead of .map.

When NOT to use this pattern:

• The framework genuinely can't express what you need — e.g. integrating a non-React widget that wants raw DOM, or a query whose shape the ORM lacks. Then drop to the imperative form, but keep it contained.
• Performance-critical low-level rendering (canvas, WebGL) — declarative wrappers exist (react-three-fiber), but raw imperative may be warranted.

Reference: React docs — Manipulating the DOM with Refs (note the "escape hatch" framing)

Replace an if/elif Chain That Returns Different Constants With a Lookup

When an if/else if chain (or a switch) does nothing but pattern-match an input to a constant output — 'small' → 4, 'medium' → 8, 'large' → 16 — the chain is a runtime impersonation of an associative array. The chain is harder to read, harder to extend (every case needs another branch), and easy to leave incomplete (a value with no branch silently returns undefined). A lookup table makes the mapping the data structure it actually is.

Incorrect (a control-flow ladder that's really a map):

function ironLevel(level: string): number {
  if (level === 'beginner') return 1;
  else if (level === 'intermediate') return 3;
  else if (level === 'advanced') return 7;
  else if (level === 'expert') return 15;
  else if (level === 'master') return 30;
  else throw new Error(`Unknown level: ${level}`);
}
// 6 lines of branching. Adding a new level → one new branch.
// Reading the mapping requires reading 6 statements.

Correct (the mapping is data, not control flow):

const LEVEL_REQUIREMENTS: Record<Level, number> = {
  beginner: 1,
  intermediate: 3,
  advanced: 7,
  expert: 15,
  master: 30,
};

function ironLevel(level: Level): number {
  return LEVEL_REQUIREMENTS[level];
}
// The table reads at a glance. New level = one new line.
// If `Level` is a literal union type, TypeScript checks exhaustiveness for you.

Variations:

• Mapping string → function (a small dispatch): Record<string, (x: T) => U> and call table[key](x).
• Mapping enum → display label: same shape; the table doubles as i18n input.
• Mapping error code → message: same shape; tests parameterise over rows.
• Mapping permission → roles allowed: Record<Permission, Role[]> and check membership.

When NOT to use this pattern:

• Each branch has different control flow (one returns, one throws, one logs) — that's real branching, not a table.
• Each branch's "constant" is actually computed from inputs not visible at table-build time — keep the function form, but consider making it a Record<K, (input) => V>.
• The chain has only two cases — a ternary is fine. Tables shine at three or more.

Pair this with a discriminated union for exhaustiveness:

type Status = 'pending' | 'active' | 'cancelled' | 'completed';

const COLOR: Record<Status, string> = {
  pending:   'gray',
  active:    'green',
  cancelled: 'red',
  completed: 'blue',
};
// If you add 'paused' to the Status union and forget the table, TypeScript errors.

Reference: Refactoring — Replace Conditional with Lookup (Martin Fowler)

Use a Recognised Tree/Object Walk Instead of Hand-Coded Recursion

When code recursively walks a tree, a nested object, a directory, or an AST, the engineer often invents a recursion structure from scratch — accumulators threaded through arguments, base cases mis-handling leaves, mutation-on-the-fly mixed with returned values. Libraries and stdlib utilities solve this once. JSON.parse + a generic walker, lodash.cloneDeepWith, walk() in Python, Object.entries recursion, or traverse in AST tools are all named tools for what feels like a one-off problem.

Incorrect (a hand-coded recursive descent over a nested config object):

function findAllUrls(obj: any, found: string[] = []): string[] {
  if (typeof obj === 'string' && obj.startsWith('http')) {
    found.push(obj);
  } else if (Array.isArray(obj)) {
    for (const item of obj) {
      findAllUrls(item, found);
    }
  } else if (obj !== null && typeof obj === 'object') {
    for (const key of Object.keys(obj)) {
      findAllUrls(obj[key], found);
    }
  }
  return found;
  // The function has three implicit jobs: walk, filter, accumulate.
  // The accumulator threading is a classic source of "I get duplicates" bugs.
  // The null check is one of the few right things — the rest is one inlined library.
}

Correct (use a generic walk; the predicate and the accumulator stay tiny):

import { traverse } from 'object-traversal'; // or equivalent

function findAllUrls(obj: unknown): string[] {
  const found: string[] = [];
  traverse(obj, ({ value }) => {
    if (typeof value === 'string' && value.startsWith('http')) found.push(value);
  });
  return found;
}
// The walker is library code, hardened. Your job is the predicate and the accumulator.

If you really must roll your own — at least separate walking from inspecting:

function* walk(node: unknown): Generator<unknown> {
  yield node;
  if (Array.isArray(node)) for (const it of node) yield* walk(it);
  else if (node && typeof node === 'object') for (const v of Object.values(node)) yield* walk(v);
}

const findAllUrls = (obj: unknown) =>
  [...walk(obj)].filter((v): v is string => typeof v === 'string' && v.startsWith('http'));
// The walker is generic and reusable. The use-site has one job: filter.

Other walks that get hand-coded:

• Directory traversal → fs.walk (Node.js 20+), os.walk (Python), find (shell).
• AST traversal → @babel/traverse, recast.visit, ts.forEachChild.
• Deep object map ({a: 1, b: {c: 2}} → {a: 2, b: {c: 4}}) → lodash.cloneDeepWith with a customiser, or write the generator once and reuse.
• React fibre walk for testing → use react-test-renderer queries, not manual children recursion.

When NOT to use this pattern:

• The tree has a very specific shape and the walk semantics are domain-specific (e.g. "stop descending if you hit a _hidden: true node, but only on the third level") — the library may not parameterise that. Inline custom recursion is fine.
• The walk is performance-critical and the library overhead is measurable — measure first.

Reference: MDN — Generators

Compose Pipelines When the Mutation-Builder Hides the Intent

This is the second level of "declarative beats imperative" — beyond replacing a single loop with .map. When a transformation has multiple steps (filter, then transform, then group, then summarise), the imperative form usually builds a mutable accumulator and threads it through a long block. The composed-pipeline form names each step. The judgment skill is recognising that a series of named operations communicates the shape of the computation — what you might call the "query" — far better than a 20-line accumulator block.

This rule is the multi-step sibling of `reinvent-stdlib-collection-ops`: that one is "the for-loop is .map"; this one is "the for-loop plus the helper function plus the early-return inside the if is a pipeline."

Incorrect (a mutation-builder hiding a four-step query):

function topCustomersByRevenue(orders: Order[]): CustomerSummary[] {
  const byCustomer: Record<string, { id: string; total: number; orderCount: number }> = {};

  for (const order of orders) {
    if (order.status !== 'completed') continue;
    if (!byCustomer[order.customerId]) {
      byCustomer[order.customerId] = { id: order.customerId, total: 0, orderCount: 0 };
    }
    byCustomer[order.customerId].total += order.total;
    byCustomer[order.customerId].orderCount += 1;
  }

  const summaries: CustomerSummary[] = [];
  for (const id of Object.keys(byCustomer)) {
    summaries.push(byCustomer[id]);
  }

  summaries.sort((a, b) => b.total - a.total);
  return summaries.slice(0, 10);
  // 14 lines. The query "completed orders, grouped by customer, top 10 by revenue" is
  // distributed across an if, an else-init, two += statements, a key-loop, and a sort+slice.
  // A reader has to assemble the pipeline mentally.
}

Correct (the pipeline reads off the page):

function topCustomersByRevenue(orders: Order[]): CustomerSummary[] {
  const completed = orders.filter(o => o.status === 'completed');
  const grouped   = Object.groupBy(completed, o => o.customerId);

  return Object.entries(grouped)
    .map(([id, custOrders]) => ({
      id,
      total:      custOrders!.reduce((s, o) => s + o.total, 0),
      orderCount: custOrders!.length,
    }))
    .sort((a, b) => b.total - a.total)
    .slice(0, 10);
  // Five named stages: filter → group → map → sort → take.
  // Each stage's purpose is the operation's name. Reordering would be a refactor, not a bug fix.
}

When the pipeline doesn't help — keep the imperative form:

// A chain that does N passes when one suffices isn't always better:
const result = items
  .filter(i => i.active)
  .map(i => transform(i))
  .reduce((s, i) => s + i.value, 0);

// vs:
const result = items.reduce((s, i) =>
  i.active ? s + transform(i).value : s, 0);
// The reduce form is one pass and one allocation. For large hot-path arrays, prefer it.
// For small arrays where readability wins, the chain is fine.

Cues for pipeline vs imperative:

Symptoms of "this should be a pipeline":

• An accumulator object/Map that's both built and later read in the same function.
• Multiple for loops on the same data, threading values through intermediate structures.
• A continue near the top of the loop — that's a .filter in disguise.
• A "post-processing" loop that walks the accumulator after the main loop.
• Comments that describe the function as a query ("get the top N customers by ...").

When NOT to use this pattern:

• The accumulation involves multi-key lookups, look-back, or running state that genuinely needs imperative flow (a state machine over the items, a state-dependent transform). Then the pipeline form contorts more than it clarifies.
• Performance-critical hot paths where intermediate-array allocation cost is measurable.
• The pipeline would require helper functions that are themselves harder to name than the original imperative block — the pipeline form should expose intent, not hide it behind small named helpers nobody else reuses.

Reference: LINQ design rationale — same idea from C#'s side; explains why pipeline composition reads as a query.

Parallelise Independent Awaits

When two or more await calls have no data dependency on each other, awaiting them sequentially turns a "max" into a "sum" of latencies. Each line looks innocent — await getUser, then await getOrders — but together they make the user wait for their sum. The mental-model gap is treating await as "I need this now" instead of "I need this before the next line that uses it." The fix is Promise.all (or .allSettled); spotting the opportunity is the judgment skill.

Incorrect (three independent fetches happening one after another):

async function loadDashboard(userId: string) {
  const user        = await api.getUser(userId);          // 120ms
  const orders      = await api.getOrders(userId);        // 200ms — could start at t=0
  const recommendations = await api.getRecommendations(userId);  // 150ms — could start at t=0
  return { user, orders, recommendations };
  // Total: ~470ms. None of these depend on each other.
}

Correct (kick them off in parallel):

async function loadDashboard(userId: string) {
  const [user, orders, recommendations] = await Promise.all([
    api.getUser(userId),
    api.getOrders(userId),
    api.getRecommendations(userId),
  ]);
  return { user, orders, recommendations };
  // Total: ~200ms (the slowest one). Same code, parallel execution.
}

Distinguishing dependent vs independent awaits:

Test: can you reorder the lines without breaking the code? If yes — they're independent.

A subtler variant: `for await` of N independent fetches:

// Incorrect (sequential, accidentally):
const results = [];
for (const id of ids) {
  results.push(await api.getItem(id));
}

// Correct (parallel):
const results = await Promise.all(ids.map(id => api.getItem(id)));

// Correct with concurrency limit (when you can't flood the upstream):
import pLimit from 'p-limit';
const limit = pLimit(5);
const results = await Promise.all(ids.map(id => limit(() => api.getItem(id))));

When NOT to use this pattern:

• The downstream service has tight rate limits — use pLimit/pMap with a concurrency cap rather than Promise.all over everything.
• An earlier call's failure should short-circuit and prevent later ones from running — Promise.all rejects fast but the others still fire. Use a guard or sequence if you specifically need that behaviour.
• The calls have shared state (write-then-read) — they're dependent even if it doesn't look that way. Keep them sequential.

Watch for `Promise.all` over an `await` that's already done — that's just `[await x, await y]` with extra steps. The fix is to remove the awaits inside `.map` so the promises start immediately:

// Wrong (no parallelism — the awaits resolve before Promise.all is even called):
await Promise.all(ids.map(async id => await api.getItem(id)));
// (Actually fine — the inner await defers within the async arrow.)

// Common mistake — calling .then sequentially in a chain:
const a = await fetch('/a').then(r => r.json());
const b = await fetch('/b').then(r => r.json());
// Still sequential. Use Promise.all on the unawaited fetches.

Reference: MDN — Promise.all

Recognise When a Custom Container Is Just a Map, Set, or Queue

When a class's job is "store things by key and let me get them back," it's a Map. When it's "track membership," it's a Set. When it's "first in, first out," a deque or array works. A surprising amount of code volume comes from Registry, Cache, Lookup, and Index classes whose entire surface area is already on Map. The class wrapper adds nothing except a name — and it hides the underlying operations so future readers can't see them.

Incorrect (a class that wraps a Map and exposes the same operations):

class UserRegistry {
  private users: Map<string, User> = new Map();

  register(id: string, user: User): void { this.users.set(id, user); }
  get(id: string): User | undefined    { return this.users.get(id); }
  has(id: string): boolean              { return this.users.has(id); }
  remove(id: string): void              { this.users.delete(id); }
  all(): User[]                         { return Array.from(this.users.values()); }
  // The class is a no-op wrapper. Every method is a rename of the Map method.
}

Correct (use the Map directly, or a type alias if you want documentation):

type UserRegistry = Map<string, User>;

const users: UserRegistry = new Map();
users.set(id, user);
users.get(id);
// Same operations, no rebuild. The type alias documents intent without writing methods.

If the class actually adds behaviour, keep it — but slim it:

class UserRegistry extends Map<string, User> {
  registerIfAbsent(id: string, factory: () => User): User {
    let u = this.get(id);
    if (!u) { u = factory(); this.set(id, u); }
    return u;
    // This method earns its place — it's not in Map. Everything else stayed Map's.
  }
}

Other cases:

• A UniqueList that "stores items only once" → Set.
• A TaskQueue whose API is push + pop_front → an array or Deque.
• A PriorityList sorted on every insert → a heap (heapq in Python, js-priority-queue).
• A Counter that tracks frequencies → new Map<K, number>() or collections.Counter.

When NOT to use this pattern:

• The class enforces an invariant the stdlib type can't — e.g. "always non-empty," "values normalised on insert." Then the class adds value. Keep it, but make it minimal.

Reference: MDN — Map