coding-standards-design

name	coding-standards:design
description	Use when designing functions, modeling data, choosing types, drawing module boundaries, or deciding what depends on what. Use when evaluating architecture, extracting abstractions, or shaping vertical slices.

Functions

• Inject deps via factory: createOperation(deps) -- pass all I/O (db, cache, email) as args to the operation function directly. createOrder({ db, emailer }), not a monolithic createOrderService that wraps all operations. Composition root = only place knowing concretions
• Extract by responsibility, not by line count — a function is "too long" when it mixes concerns (orchestration + low-level parsing + I/O + formatting), NOT when it crosses a line threshold. A 50-line linear workflow that reads top-to-bottom as "step 1 → step 2 → step 3" is BETTER than ten 5-line helpers the reader has to jump between. Splitting by line count produces "spaghetti of indirections". Extract a helper ONLY when: (a) it's used 3+ times (Rule of Three), (b) it's at a genuinely different abstraction level than the caller (e.g., validateOrder(order) then a 20-line inline regex parser), OR (c) it's independently testable as a unit. No line-count ceiling. Pure by default. SRP. Reviews: function split into 5+ helpers each called once from one place -> flag "inline back, this is over-fragmented"; function mixing I/O orchestration with inline byte/regex/date manipulation -> flag "extract the low-level piece"
• One level of abstraction per function -- don't mix high-level orchestration with low-level details. A function that calls validateOrder(order) then inlines a regex to parse a date string is mixing levels. Reviews: function body mixing domain calls with raw string/regex/byte manipulation -> flag "extract low-level detail to named helper"
• Single call site = inline -- a function with exactly ONE caller anywhere in the codebase is indirection without benefit. Paste the body at the call site, delete the function. Applies regardless of location: same file, helper file, shared/, utils/. The reader shouldn't jump to a named helper just to discover it's specific to its caller. Exceptions (must be real, not hypothetical): (a) step-down orchestrator helper named by business intent (applyPromotions, convertCurrency) that lets the caller read as a top-level map, (b) function has dedicated unit tests, (c) function lives inside a slice-folder as an isolated independently-testable concern. Reviews: function with exactly 1 call site + no dedicated test + not a business-named step-down helper -> flag "inline at call site, delete the function"
• Stepdown Rule — orchestrator first, details below -- write the top-level function first (names each step at business level), then its helpers below in the same file. Reader follows intent top-to-bottom, reads details only when needed. This is why a long well-structured file beats five small scattered ones: top = map, bottom = territory.
• Max 3 positional args; objects to prevent same-type swaps -- options object for 4+ args. Also use objects when 2+ consecutive params share the same type — positional same-type args compile even when swapped: sendEmail("Welcome!", "Hi there") silently inverts subject/body. Reviews: 2+ consecutive params of same type -> flag "use named object to prevent silent swap"
• CQS -- command OR query. Composition over inheritance
• Focused modules -- no common/shared grab-bags
• No default parameters -- use explicit factory methods -- default params hide behavior and create invisible coupling. createUser(name, role = 'viewer') -> createViewer(name) / createAdmin(name). Each factory is self-documenting and independently testable. Reviews: function with default params controlling behavior -> flag 'extract named factory'
• No flag arguments -- split the function -- a boolean (or enum) parameter that makes the function do two fundamentally different things is two functions wearing one signature: render(node, isPreview) -> render(node) / renderPreview(node). The caller passing a literal true/false never reads better than two named calls. (Same root as "no default parameters" above: a param that switches behavior is hidden control flow.) Reviews: boolean/enum param gating two distinct code paths -> flag "split into named functions"

Data & Types

• Immutable -- map/filter/reduce/spread
• Extract literals to named constants
• Strict typing everywhere
• Data clumps -> extract Value Object -- when the same 3+ fields appear together across multiple functions, parameters, or types (street, city, zipCode, country or amount, currency), extract a Value Object. Reviews: same 3+ fields repeated in 2+ locations -> flag "extract Value Object"
• Externalize config -- rates, thresholds, multipliers in config objects, not inline. Business params changeable without editing function bodies
• Bound every input -- reject, always reject -- every external param validated and rejected if invalid. weight must be > 0 && < MAX_WEIGHT or return Result error. NEVER ?? defaultValue, ?? 0, || fallback for invalid inputs -- silent-failure bugs. Only correct response: return Result error. Reviews: ?? value or || default on external input -> flag as bug
• Boundary condition awareness -- every function handling collections, indices, or numeric ranges must address: null/undefined input, empty collection, single element, off-by-one at boundaries, numeric overflow/underflow. Reviews: loop or index access without boundary check -> flag
• index, count, and size are distinct types -- same primitive (number/u64), different semantics. index is 0-based, count is 1-based, size is count × unit_size. To go from index to count, add one. Treat conversions deliberately: never let an index flow into a function expecting a count. The usual off-by-ones live at these joints. Reviews: arithmetic on index/count without an explicit conversion comment -> flag "name the conversion"
• Show your intent with division -- never write ambiguous a / b in domain logic. Use divExact, divFloor, divCeil (or wrap with named helpers) to tell the reader you've thought about rounding. Reviews: division in domain code without explicit rounding intent -> flag
• Data vs Object — choose deliberately for every type -- pick one before writing fields:
  • Data: value equality, immutable, internals exposed, fixed shape with open operations (records, DTOs, events, action descriptions). Default for anything that moves between modules.
  • Object: identity-based equality, usually mutable, encapsulated, fixed operations with open shape (handles, sessions, sockets). Reserved for things with lifecycle.
  • Most "immutable objects with methods" want to be Data. The giveaway: if you'd be happy to serialize, diff, or hash it, it's Data — stop hiding it behind a class. Reviews: class with only getters and pure methods on internal fields -> flag "this is data, expose the fields"
• Don't generalize the problem you don't have -- generalization replaces a known specific problem with a harder, vaguer one. Don't add a parameter, type variable, or hook for an imagined second caller. The Rule of Three applies before you generalize, not after the first instance. Symmetric exception: if the second case is already concrete (spec written, ticket open), generalize now — but name it in the commit, not "future-proofing". Reviews: type variable / config slot / strategy interface with one user -> flag "specialize back; the second case will tell us its real shape"

Type design — make illegal states unrepresentable

• The one question — push every invariant into the types -- for every type you add or change, enumerate the states it permits, then ask which the domain forbids. Each permitted-but-forbidden state is a design bug; restructure so it can't be expressed. A state the compiler rejects needs no runtime check, no test, no "should never happen" comment. Proportionality: act only when the bad state is realistically reachable AND would cause a real bug / force defensive checks AND the fix is cheap and lives in domain code — the test is "name the forbidden state this prevents"; if you can't, leave it. Free text (note, description) is just string; a booking that's both paid and cancelled (refund pending) is a real state — merging it loses information. Reviews: a representable state the domain forbids -> flag with the union/brand/parser that removes it
• Sum types over flag/optional bags -- mutually-exclusive booleans (isLoading+isError+isSuccess) or optionals (error? and data?) make nonsense states representable (2^n combos, most meaningless). Use one discriminated union with a status discriminant, one variant per real state, carrying exactly that state's fields. Reviews: 2+ booleans/optionals that can't all be true at once -> flag "collapse into a discriminated union"
• Exhaustiveness with never -- every switch/match over a union ends default: return assertNever(x) so a later-added variant is a build failure, not a silent fallthrough. Reviews: exhaustive-looking switch with no assertNever/never-default -> flag "add exhaustiveness guard"
• Parse, don't validate -- a validator returning boolean/void throws the knowledge away; every caller must re-trust it. Parse at the boundary into a proof-carrying type and pass that inward (parseEmail(s): Email | undefined, then sendInvite(email: Email) — the type is the proof). Prefer the project's existing schema lib (Zod/Valibot/@effect/schema/…) over a hand-rolled guard; never add a dependency just to satisfy this. Reviews: validation returning boolean/void then passing the raw primitive onward -> flag "parse into a typed value at the boundary"
• Brand domain primitives -- structural typing makes UserId, OrderId, Email, money, units all interchangeable string/number — typos compile. Brand them (type UserId = string & { readonly __brand: "UserId" }), minted only by a parser/constructor. Don't brand a value with no invariant to prove. (Generalizes the index/count/size rule above.) Reviews: distinct domain ids/quantities passed as bare string/number where a swap would silently compile -> flag "brand the type"
• as / ! are unchecked — justify each in the diff -- ✅ as const, ✅ x satisfies T, ✅ an as/! immediately after a real runtime check you can see; ❌ JSON.parse(body) as User, resp as SuccessResponse, maybeUser!.name — assertions of facts nothing verified. Reviews: as/! with no nearby justifying check -> flag "replace with a type guard or parser"

Architecture

• Vertical slices — organize by domain/operation, not by role -- structure: src/features/{domain}/{operation}.ts. Each operation file owns its full vertical: handler, business logic, DB query, types — everything that changes together for that use case. Never split by technical role (services/, repositories/, handlers/) on day one. Read the feature = open one file and scroll. Reviews: new feature split by role across multiple directories -> flag "collapse into a feature slice"
• Domain shared — extract on second real usage, never by anticipation -- a helper stays in the operation file until a second operation in the same domain genuinely needs it. Then extract to features/{domain}/shared/. Never create a src/shared/ for domain business logic — only for purely technical utilities (date formatting, pagination) used across 3+ unrelated domains. Reviews: logic in shared/ with only one caller -> flag "inline back, premature extraction"
• Cross-domain access — public API only -- a slice imports from another domain's public exports (features/inventory/index.ts) only, never from its internal files or its DB tables directly. 1-2 external domains: direct import of their public use case. 3+ domains react to the same operation: emit a domain event (OrderCreated) and let each domain react independently. Reviews: import from features/X/internal/ or direct DB query into another domain's tables -> flag "use X's public API"
• Store ownership (frontend) -- each domain owns its store (features/cart/store.ts). Other domains read via exported selectors. Write via explicitly exported actions only — never mutate another domain's state directly. Reviews: domain mutating state it doesn't own -> flag "call the owning domain's exported action"
• Factory DI -- createOrder({ db, emailer, logger }). Per-operation, plain functions, framework-free. No monolithic service objects wrapping all operations of a domain
• Crosscutting via middleware -- crosscutting concerns (tracing, logging, auth) belong at the HTTP middleware layer, not inside operation logic. No logger.info() in business logic — log at handler entry/exit. Reviews: logging or tracing inside operation business logic -> flag "move to middleware"
• Structured API errors -- { type, code, status, detail } not bare { message } strings (see coding-standards:errors)
• Map DB entities to DTOs -- dedicated response types for API outputs. Reviews: raw DB entity returned -> flag "missing DTO mapping"
• API-first -- define schema (OpenAPI, route schema) BEFORE handler. Zod is complementary but not api-first. Contract must exist as standalone artifact. Reviews: handler without schema -> flag "missing API contract". For full API design, see api-design skill.
• Shotgun Surgery detection -- when a single logical change requires edits in 5+ unrelated files, the responsibility is scattered. Complementary smell: Divergent Change -- one module changes for multiple unrelated business reasons (e.g. a CustomerModule that changes when billing rules change AND when loyalty program changes AND when address format changes — three distinct business domains sharing one file). "Business reasons" means distinct domain concerns (billing, loyalty, shipping, auth), NOT technical refactors or bug fixes. Reviews: change touching 5+ files for one concept -> flag "consolidate into one module"; one module changing for 2+ unrelated business domains -> flag "split by business responsibility"
• Negative-space module design — every module answers three "must NOT" questions -- before writing a module, name explicitly:
  1. What must this module NOT expose? (which fields/types/methods must never leak into its public surface)
  2. What must this module NOT depend on? (which other modules must stay invisible from inside)
  3. What must NOT depend on this module? (which modules must never import it, even indirectly) The interesting design content lives in the prohibitions, not the features. A module that doesn't answer these three is under-designed; the answers are also the lint rules to enforce. Pairs with "encapsulate one changeable assumption per module" (next rule). Reviews: new module without an explicit non-dependency policy at the top of its public file -> flag "declare what this module is NOT"
• One module = one absorbed assumption -- Parnas: each module's job is to absorb a single change that we predict will happen. Name the assumption: "this module owns the wire format for X", "this module hides the choice of cache backend", "this module is the only place that knows the audit schema". When that assumption changes, only this module changes. A module without a nameable absorbed assumption is just a folder.
• Public vs private dependencies -- a dependency is public if its types appear in your module's parameters or return types (the consumer must know about it to call you). It is private if you only use it internally (the consumer never sees it). Audit every export: "to call this, what other modules' types must the user import?" Each one is a public dep — a transitively imposed cost. Minimize publics. Reviews: function exposing a third-party type in its signature when a domain wrapper would do -> flag "private it; wrap the type at the boundary"
• Don't split a module just for size -- splitting trades linguistic complexity (long file, gettable with "find references", refactors atomic) for system complexity (wiring, import graph, ordering constraints, build coupling). The first scales with tooling; the second doesn't. Split when responsibilities are genuinely distinct, not when the line count is high. Reviews: file split that didn't add a second consumer or a different reason-to-change -> flag "merge back; this is system complexity for no payoff"
• Behavior, not ontology, decides what belongs together -- "is a contractor really a kind of employee?" / "do these two functions belong together?" are empty questions. Ontology categories live in human heads, not in the program. Ask instead: "must these behave the same in code?" or "do they change for the same reason?". If yes, group; if no, separate. Reviews: design debate framed around "is X really a Y" -> redirect to "what should X and Y do in code? do they share that?"
• Single root pointer for state -- a program's reachable state should hang off one root that's passed down explicitly (no globals, no singletons). Component-level handles are subtrees of the root. Benefits: the debugger can dump every live resource by walking one pointer, readers can find any state by tracing types, and tests instantiate one root instead of a constellation of fixtures. Reviews: state-bearing component reached via a global / module-level singleton / framework magic -> flag "expose via root pointer"
• Stored callbacks are an undebuggability tax -- thing.on_event(cb => ...) turns "what happens next when event fires" into "literally anything stored in the registry, in registration order". Prefer pull-based event loops (while (const ev = thing.next()) { ... }) where the next step is right there in the source. Same concern with async/await making the call stack invisible. Use stored callbacks only when the registry is intrinsic to the domain (DOM events, real plugin systems) and document the dispatch order. Reviews: .on(...) / .subscribe(...) / .addListener(...) for internal eventing where a pull loop would work -> flag "convert to explicit polling"
• Reify actions as data when state changes span multiple objects -- instead of a function that directly mutates several entities, return a small action description (a value: an enum, a tagged union, a list of operations) from pure code, and have a tiny interpreter execute it. Turns multi-entity state changes into pure functions, concentrates the mutation surface, and enables centralized checks (permissions, dry-run, audit log, replay) that no caller can bypass. Caveat: one command class per action is the degenerate form — you've renamed methods, not reduced surface. The point is a small composable action type covering many actions (the way SQL covers many queries).
• Bound everything that loops, queues, or retries -- every loop has a max iteration count, every queue has a max depth, every retry has a max attempt, every buffer has a max size. When a loop is genuinely unbounded (server accept loop, scheduler tick), assert it (assert(consecutive_empty_polls < MAX_BACKOFF)) and log when nearing the bound. Unbounded loops are how production hangs; unbounded queues are how memory dies; unbounded retries are how cascades start. Reviews: while (true), for(;;), .retry(), .push() on a queue without a documented ceiling -> flag "name the bound"
• Run at your own pace — don't directly react to external events -- if your program is driven by external events (webhooks, queue messages, sensor data), don't process each as it arrives. Tick at a known frequency, drain the inbox during the tick, batch. This bounds rates, enables backpressure, makes timing properties testable, and lets you batch I/O. The reactive shape ("on every event, do X") inverts control and makes the system's pace depend on the outside world. Reviews: handler that does meaningful work synchronously on each event with no tick boundary -> flag "drain on tick, don't react per event"
• Many small functional cores, not one giant one -- the functional-core/imperative-shell pattern scales by replication, not by growing the core. A 30,000-line single core wrapped by a giant shell is unmanageable; many small (core, shell) pairs communicating via values is. When in doubt, split the core along the lines of "what state cohesively changes together" — if cursor and selection always move together, they're one core; if not, two. Bad cohesion grouping defeats the pattern.
• Module depth over shallow wrappers -- a module's interface should be simpler than its implementation. Pass-through methods that forward calls with identical signatures add no value. Reviews: public API as complex as internal logic -> flag "shallow module, merge or deepen"; method forwarding to another with same signature -> flag "pass-through, inline or add value"
• Information leakage detection -- when the same design decision (file format, protocol details, serialization logic) is duplicated across modules, a change forces edits everywhere. This is knowledge duplication, distinct from code duplication. Reviews: same format/protocol knowledge in 2+ modules -> flag "information leakage, encapsulate in one module"
• Security review checklist in reviews -- every code review must check: no exposed secrets/credentials, input validation on all external boundaries, authorization checks on all state-changing operations, no PII in logs, no SQL/XSS injection vectors. Reviews: state-changing endpoint without auth check -> P0 blocker