code-quality

Sub-Skill: Code Quality & Common Mistakes

Purpose: Prevents the 20 % of mistakes that cause 80 % of review comments. Concrete rules from real projects — no boilerplate.

Rule classification

• MUST — load-bearing. Violating causes bugs, security issues, or cross-team confusion. Never break.
• SHOULD — default behavior. Deviation needs a documented reason in the code or PR.
• AVOID — usually wrong; documented exception inline where needed.

Where these rules don't strictly apply: test fixtures, generated code (parser tables, codegen output, schema-derived types), CLI bootstrap scripts, framework adapters, and migration scripts may legitimately differ. The rules below apply to production code paths unless an inline exception says otherwise.

---

Rules

Functions & Logic

1. SHOULD: Functions under ~30 lines. Longer → split where practical. Reduces cognitive load.
2. AVOID: Boolean as last argument. render(true) is unreadable. Use enum or named-property: render({ withHeader: true }).
3. SHOULD: Early return over nested if-blocks. Nesting depth > 2 is a warning sign.
4. SHOULD: No magic numbers. if (status === 3) → if (status === OrderStatus.SHIPPED).
5. AVOID: Double negation. if (!isNotValid) → if (isValid).

Variables & Naming

1. SHOULD: Variable names describe content, not type. userList instead of arr, activeUserId instead of id.
2. SHOULD: Temporary variables for complex expressions. const isEligible = age >= 18 && !isBanned; instead of cramming it all into an if.
3. AVOID: Abbreviations except established ones (id, url, ctx, req, res). usr, cfg, tmp → spell out.

Error Handling

1. MUST: Every async call needs try/catch or .catch(). Unhandled promise rejections crash Node processes.
2. MUST: Never silently swallow error objects. catch (e) {} is forbidden. At minimum: logger.warn(e).
3. MUST: Error messages must include context. throw new Error('User not found: ' + userId) not throw new Error('Not found').
4. SHOULD: Catch specific exception types, not broad categories. Catch ConnectionError not Exception. Broad catches mask unrelated bugs and make debugging harder.
5. SHOULD: Use exponential backoff with jitter for retry logic. Never retry in a tight loop. Each retry should wait longer than the last with randomization to prevent thundering herds.
6. MUST: Never retry non-idempotent operations without explicit safeguards. A retried POST that creates a resource can produce duplicates. Use idempotency keys or check-before-retry patterns.
7. SHOULD: Propagate errors with context rather than swallowing and re-raising generic exceptions. Use raise NewError("context") from original_error to preserve the chain. Reference: ERR-2026-016.