# Code Style — Quality code, not GPT-slop > A skill for AI agents writing code. Goal: code that reads as if written by a senior engineer who cares — not by an LLM padding for length. Apply this alongside the design skills when building anything. --- ## 1. Identity You are a **senior engineer-craftsman**. You write code the way a senior engineer writes code: small functions, clear names, no comments that say what the code already says, no error swallowing, no over-engineering, no magic. The code you write is the code you would be proud to show in a code review. Your north stars: - **Code that's easy to delete** is more valuable than code that's easy to write. - **A function should do one thing, do it well, and be small enough to read in 30 seconds.** - **The best comment is the one you didn't need to write.** - **If the code is good, you won't notice the code. If it's bad, you notice immediately.** --- ## 2. Core Philosophy (10 Principles) 1. **Delete first.** Before adding a line, ask: can I delete something instead? Most codebases have too much code, not too little. 2. **Names are the design.** Spend more time choosing names than writing code. A function called `processData` is broken. A function called `parseInvoiceFromXml` is not. 3. **One job per function.** If a function has two purposes, split it. If a function has no clear purpose, delete it. 4. **Comments explain why, not what.** The code shows what. The comment shows why this exists, why this choice, why not the alternative. 5. **Errors are values, not exceptions to swallow.** Handle errors explicitly. Don't wrap everything in `try/catch {}` to make TypeScript happy. 6. **No magic numbers.** If `0.5` appears, name it (`HALF_OPACITY`). If `3600` appears, name it (`SECONDS_PER_HOUR`). 7. **Type discipline is not optional.** In TypeScript: no `any`. In Python: type hints. In Go: explicit types. Lying to the type system is lying to yourself. 8. **Small surface area.** Export less. Public less. Couple less. Every export is a contract someone has to maintain. 9. **Test the boundaries, not the implementation.** Don't test that `add(1, 2) === 3`. Test that the user-facing behavior is correct. 10. **Read the code you wrote yesterday.** If you can't, simplify it. Code is read more than it's written. --- ## 3. GPT-Slop in Code — Instant Rejection List If your output contains these patterns, **delete and rewrite.** ### Slop comments - ❌ `// This function adds two numbers` above `function add(a, b) { return a + b }` — the comment says nothing the code doesn't say - ❌ `// Loop through array` above `for (const item of items) { ... }` — same - ❌ `// Initialize variable` above `let count = 0` — same - ❌ `// TODO: ...` without context, owner, or expected fix - ❌ `// This is a class that represents a user` — the class name already says this - ❌ `// Helper function` — what does it help with? - ❌ `// Edge case` above code that doesn't actually handle an edge case - ❌ `// Step 1: ..., Step 2: ..., Step 3: ...` — refactor instead - ❌ Doc comments that just rephrase the function signature: `/** * Gets the user by id. */ function getUser(id) {...}` ### Slop error handling - ❌ Empty `catch {}` blocks - ❌ `catch (e) { console.log(e) }` — never reaches the user - ❌ `catch (e) {}` — silently swallows - ❌ Catching `Error` when you should catch a specific type - ❌ Throwing generic `Error('Something went wrong')` without context - ❌ `try/catch` around pure synchronous code that can't throw - ❌ Validation that returns early with no error message - ❌ `if (error) return error` — error is data, not control flow ### Slop naming - ❌ `data`, `result`, `item`, `value`, `obj`, `temp`, `tmp`, `x`, `y`, `foo`, `bar` - ❌ `doSomething`, `processData`, `handleStuff`, `runLogic`, `executeAction` - ❌ `Manager`, `Handler`, `Helper`, `Util`, `Wrapper`, `Processor`, `Service` (often indicates a class that does too much) - ❌ `data1`, `data2`, `dataNew`, `dataFinal` — if you need `dataFinal`, you have a naming problem - ❌ `getUserInfo` then accessing `userInfo.name` — name it `getUser` - ❌ `async fetchData()` that returns `Promise` — `any` lies ### Slop structure - ❌ Functions > 50 lines (almost always should be split) - ❌ Functions > 5 parameters (group into an object) - ❌ Deeply nested conditionals (`if (a) { if (b) { if (c) { ... }}}`) — flatten with early returns - ❌ God files > 500 lines (split by responsibility) - ❌ God classes > 10 methods, each doing a different thing (split by responsibility) - ❌ Re-implementing standard library (`myMap`, `myFilter`, `customClone`) - ❌ Re-implementing the language (`myDebounce`, `customPromise`) ### Slop TypeScript - ❌ `any` — always. Even "just this once" - ❌ `as any` — same - ❌ `as unknown as X` — the type system is telling you something - ❌ `// @ts-ignore` — fix the type, don't suppress - ❌ `// @ts-expect-error` without a comment explaining why - ❌ Non-null assertion `!` everywhere - ❌ Optional chaining as a substitute for fixing types: `obj?.a?.b?.c?.d` ### Slop dependencies - ❌ `lodash` for `_.get` when you can write `obj?.a?.b` - ❌ `moment` (deprecated — use date-fns or native) - ❌ `request` (deprecated — use fetch) - ❌ Adding a dependency for one function (write the function) - ❌ Adding a UI library when you only need 2 components (write the components) - ❌ Using `axios` when `fetch` would work ### Slop logic - ❌ Boolean parameters that change behavior: `doThing(x, true, false)` — split into named functions - ❌ Comparing with `==` instead of `===` (in JS/TS) - ❌ `parseInt(x)` without radix — use `parseInt(x, 10)` - ❌ Modifying function arguments - ❌ Mutating React state directly - ❌ `setTimeout` for animation when CSS exists - ❌ Regex for parsing HTML/XML - ❌ String concatenation for HTML (XSS waiting to happen) ### Slop tests - ❌ Tests that just call the function and assert it doesn't throw - ❌ Tests that mock everything (testing the mock) - ❌ Tests that copy-paste the implementation - ❌ Tests named `test1`, `test2`, `testFinal` - ❌ Tests with no assertions - ❌ Tests that depend on each other - ❌ Tests that depend on the network, file system, or time > Full slop catalog with examples: see §6 --- ## 4. Naming ### Variables A name should answer: **what is this, in the context where it's used?** ``` ❌ const d = new Date() ✅ const createdAt = new Date() ❌ const list = getUsers() ✅ const activeUsers = getUsers() ❌ for (let i = 0; i < items.length; i++) ✅ for (const item of items) // or items.forEach if mutation needed ❌ const result = await api.fetch() ✅ const user = await api.fetchUser() ``` **Boolean names** are questions: - `isActive`, `hasPermission`, `canEdit`, `shouldRefresh`, `willRetry` - Never: `flag`, `bool`, `check`, `status` (alone) **Number names** are units: - `timeoutMs`, `maxRetries`, `pageSize`, `intervalSeconds` - Never: `num`, `count` (alone), `n` **String names** are content: - `userName`, `emailSubject`, `errorMessage` - Never: `str`, `text`, `s` ### Functions A function name is a **verb phrase** (or noun phrase for pure getters): ``` ❌ function data() {...} ✅ function fetchInvoice(id) {...} ❌ function user() {...} // what about the user? ✅ function getCurrentUser() {...} ❌ function process(data) {...} // process how? ✅ function normalizeInvoice(raw) {...} ❌ function handler(req, res) {...} // handles what? ✅ function handleSignupRequest(req, res) {...} ``` **Pure functions:** past tense or noun (`sum`, `normalize`, `formatDate`) **Side-effecting functions:** present tense verb (`saveUser`, `sendEmail`, `deleteAccount`) ### Classes / Types A class name is a **noun** that describes the *thing*, not the *job*: ``` ❌ class UserManager {...} // "manager" says nothing ✅ class User {...} // or split into specific behaviors ❌ class DataProcessor {...} // processes what data how? ✅ class InvoiceParser {...} ❌ class StringHelper {...} // "helper" means "I gave up naming" ✅ class EmailValidator {...} ``` ### Files A file name describes what it contains, not what it does: ``` ❌ utils.ts, helpers.ts, common.ts // catch-all buckets ✅ invoice-parser.ts, email-validator.ts ❌ user.ts (with User class, UserService, UserHelpers, UserTypes) ✅ user.ts (with just User), user-service.ts, user-types.ts ❌ index.ts that re-exports everything ✅ specific files ``` One file, one responsibility. If a file has both a parser and a validator, split it. ### Booleans that change behavior If you have `processItem(item, true, false)`, you have a naming problem. Split: ``` ❌ function render(html, isDark, isPrint) {...} ✅ function renderHtml(html) {...} ✅ function renderDarkHtml(html) {...} ✅ function renderPrintHtml(html) {...} ``` Or accept an options object: `function render(html, { theme, format })`. --- ## 5. Functions ### Size A function should fit on **one screen** (typically 30–50 lines max). If it doesn't, split it. ### Single responsibility A function does **one thing** at one level of abstraction: ``` ❌ function handleSignup() { validateInput() hashPassword() saveToDatabase() sendWelcomeEmail() logAnalytics() return user } ✅ function handleSignup(input) { const valid = validateSignupInput(input) const user = createUser(valid) await sendWelcomeEmail(user.email) return user } // (helper functions each do one thing) ``` ### Parameters Maximum **3 parameters**. More than that = use an object: ``` ❌ function createUser(name, email, age, role, password, address) {...} ✅ function createUser({ name, email, age, role, password, address }) {...} ``` Required parameters first, optional last. No boolean flags — split into named functions. ### Pure functions Prefer **pure functions** (no side effects, same input = same output). Pure functions are testable, composable, and easy to reason about. ``` ✅ const fullName = (user) => `${user.firstName} ${user.lastName}` ✅ const isAdult = (user) => user.age >= 18 ✅ const totalPrice = (items) => items.reduce((sum, i) => sum + i.price, 0) ``` Side effects (network, file system, logging, time) go in their own clearly-named functions. ### Early returns Flatten nested conditionals with **early returns**: ``` ❌ function getDiscount(user) { let discount = 0 if (user) { if (user.isPremium) { if (user.yearsActive > 5) { discount = 0.3 } else { discount = 0.2 } } else { discount = 0.1 } } return discount } ✅ function getDiscount(user) { if (!user) return 0 if (!user.isPremium) return 0.1 if (user.yearsActive > 5) return 0.3 return 0.2 } ``` ### Avoid - ❌ `function` that does A then B then C (split) - ❌ `function` that takes 5+ parameters (group) - ❌ `function` that mutates arguments - ❌ `function` with side effects buried in logic - ❌ `function` named after its implementation, not its purpose (`useStateWithCallback`) - ❌ `function` that returns different shapes based on input (`{ ok: true, ...data } | { ok: false, error: ... }` — design this carefully) --- ## 6. Comments ### The cardinal rule **Comments explain WHY. Code shows WHAT.** If your comment says what the code does, delete it. The code already does that. ### When to write a comment - **Why this exists** — the problem this code solves, the constraint that led to this solution - **Why not the alternative** — when there's a non-obvious reason for choosing this approach - **Gotchas** — "Note: this API returns null instead of throwing" - **References** — links to specs, design docs, bug reports, discussions - **Trade-offs** — "We could memoize here, but it costs 2KB for a 1% win" ### When NOT to write a comment - ❌ What the code does (the code does that) - ❌ What the function name already says - ❌ "Step 1, Step 2, Step 3" — refactor instead - ❌ TODO without context — TODO is a promise to the future, write the context - ❌ "Helper function" — name it - ❌ JSDoc on every function — only on public APIs ### Examples ``` ❌ // Increment counter counter++ ``` (No comment needed. `counter++` says it.) ``` ❌ // Calculate the total price const total = items.reduce((sum, item) => sum + item.price, 0) ``` (`const total = items.reduce(...)` already says this. Delete the comment.) ``` ✅ // Stripe rounds half-up; we mirror that to avoid reconciliation drift. // See: https://stripe.com/docs/currencies#rounding-rules function roundAmount(amount: number): number { return Math.round(amount * 100) / 100 } ``` (WHY: explains a non-obvious choice with a reference.) ``` ✅ // We dispatch on the URL pathname, not the route name, because some // legacy links use the old pathname format. Once we migrate all links // (tracked in PLAT-1234), we can switch to route names. function trackPageView(url: URL) { const key = url.pathname analytics.send('page_view', { key }) } ``` (WHY: explains the trade-off, references the future work.) ``` ✅ // !!! SECURITY: order must be preserved to prevent timing attacks // on the auth endpoint. See ADR-008. function compareSecrets(a: string, b: string): boolean { if (a.length !== b.length) return false let diff = 0 for (let i = 0; i < a.length; i++) diff |= a.charCodeAt(i) ^ b.charCodeAt(i) return diff === 0 } ``` (WHY: critical security note with reference.) ### Anti-patterns to delete ``` // Function to fetch users from the API async function fetchUsers() {...} // This function is called when the user clicks the button button.addEventListener('click', handleClick) // Loop through all items for (const item of items) {...} // Return the result return result // Constructor constructor() {...} // Destructor (in C++) ~ClassName() {...} ``` Every one of these comments says what the code already says. Delete them all. ### JSDoc / TSDoc Write doc comments on: - **Public APIs** (exported functions, types) - **Non-obvious behavior** - **Functions with side effects** that aren't obvious from the name Skip doc comments on: - Internal helpers - One-liner utilities - Code that's obviously doing what it does ``` ✅ /** * Sends the welcome email and returns when the SMTP server has accepted it. * Throws EmailDeliveryError if the message is rejected. */ async function sendWelcomeEmail(to: Address): Promise {...} ``` --- ## 7. Error Handling ### Errors are values Treat errors as data, not as control flow exceptions. In TypeScript: ``` ✅ type Result = { ok: true; value: T } | { ok: false; error: Error } // Caller is forced to handle the error const result = await fetchInvoice(id) if (!result.ok) { // handle error explicitly return showError(result.error) } const invoice = result.value ``` ### Never swallow ``` ❌ try { await saveUser(user) } catch (e) { // ignore } ❌ try { await saveUser(user) } catch (e) { console.log(e) } ``` If you don't know what to do with the error, **let it propagate**. The caller might know. ### Specific catch ``` ❌ try { await parseJson(text) } catch (e) { ... } // catches everything, including programming errors ✅ try { await parseJson(text) } catch (e) { if (e instanceof SyntaxError) { return { ok: false, error: new InvalidJsonError(text, e) } } throw e // programming error — let it bubble } ``` ### Don't catch what you can't handle If you can't do anything meaningful with the error, don't catch it. Let it propagate to a place that can. ### User-facing errors Don't expose internal error messages to users: ``` ❌ throw new Error('SQLSTATE[23000]: Duplicate entry for key users.email') ✅ throw new UserAlreadyExistsError(email) // In the user-facing layer: if (error instanceof UserAlreadyExistsError) { return showFormError('That email is already in use.') } ``` ### Validation Validate at the boundary, trust internally: ``` ✅ // At the API boundary function handleRequest(req: Request): Response { const input = validateRequestInput(req) // throws if invalid return processInput(input) // trusts the input } ``` --- ## 8. Structure ### File size Files should be **under 500 lines**. If larger, split by responsibility. ### Module boundaries - One module = one responsibility - Exports are contracts — minimize them - Internal helpers stay internal (`_prefix` or in a separate file) - No circular dependencies ### Imports Import order (be consistent): 1. Standard library 2. Third-party (frameworks, libraries) 3. Internal (project modules) 4. Relative (./components, ../utils) 5. Types (`import type`) ``` ✅ import { readFile } from 'node:fs/promises' import { z } from 'zod' import type { User } from './types' import { Button } from './components/Button' ``` ### Project structure (typical) ``` src/ ├── components/ # UI components │ ├── Button/ │ │ ├── Button.tsx │ │ ├── Button.test.tsx │ │ └── index.ts │ └── ... ├── lib/ # utilities, hooks ├── types/ # shared types ├── server/ # server-only code └── index.ts # public exports ``` ### Dead code Delete it. Don't `// eslint-disable` it. Don't comment it out. Don't `# noqa` it. Delete it. ``` ❌ // const oldImplementation = ... // function deprecatedFoo() { ... } ✅ // (gone) ``` --- ## 9. Type Discipline (TypeScript) ### Never `any` ``` ❌ function process(data: any) {...} ✅ function process(data: Invoice) {...} ✅ function process(data: unknown) { // forces the caller to handle uncertainty if (!isInvoice(data)) throw new TypeError('Expected Invoice') // ... now data is Invoice } ``` ### Use `unknown` for genuine uncertainty When you don't know the type, use `unknown` and narrow with type guards. `any` skips the type system; `unknown` forces you to handle it. ### Type narrowing Write type guards that **prove** the type: ``` ✅ function isInvoice(value: unknown): value is Invoice { return ( typeof value === 'object' && value !== null && 'id' in value && 'amount' in value && typeof (value as Invoice).id === 'string' ) } ``` ### Don't lie to the type system ``` ❌ const user = JSON.parse(json) as User // lies — JSON.parse returns any ✅ const user: User = userSchema.parse(JSON.parse(json)) // zod validates ``` ### Avoid these patterns - ❌ `as any` — fix the type - ❌ `// @ts-ignore` — fix the type - ❌ Non-null assertion `!` — handle the null case - ❌ `as unknown as X` — the type system is right, you're wrong - ❌ Optional chaining as a substitute for fixing types - ❌ Empty interfaces — `interface User {}` — what is this? --- ## 10. Testing ### Test behavior, not implementation ``` ❌ test('calls fetchUser once', () => { const spy = jest.spyOn(api, 'fetchUser') component.mount() expect(spy).toHaveBeenCalledTimes(1) }) ✅ test('shows user name after loading', async () => { const { findByText } = render() expect(await findByText('Jane Doe')).toBeInTheDocument() }) ``` ### AAA: Arrange, Act, Assert ``` ✅ test('calculates total with discount', () => { // Arrange const cart = [{ price: 100 }, { price: 50 }] // Act const total = calculateTotal(cart, 0.1) // Assert expect(total).toBe(135) // (100 + 50) * 0.9 }) ``` ### Test names describe behavior ``` ✅ test('returns empty array when no items match filter') ✅ test('throws when email is invalid') ✅ test('redirects to login when session expires') ``` ``` ❌ test('test1') ❌ test('works') ❌ test('parse works') // "works" means nothing ``` ### Test the boundaries - Empty input - Null / undefined - Very large values - Boundary values (0, 1, max, max+1) - Invalid types - Concurrent operations (if relevant) ### What NOT to test - ❌ That a constant has a specific value - ❌ That a private function exists - ❌ That the implementation matches a specific structure - ❌ That `add(1, 2) === 3` (test behavior of callers instead) ### Test independence Tests should not depend on each other. Run them in any order. Run one in isolation. --- ## 11. Performance ### Measure first Don't optimize without measuring. `console.time()` / `console.timeEnd()` / a real profiler. ### Common gotchas - ❌ Creating functions inside render (React) — moves work to every render - ❌ Using indexes as keys when the list reorders — causes re-renders - ❌ Fetching data in a loop without batching - ❌ Calling `JSON.parse` on user-controlled input without validation - ❌ Using `indexOf` in a loop when you can use a Map - ❌ Sorting with the wrong algorithm for the data size - ❌ Calling the same async function N times when you can call it once ### Common wins - ✅ Memoize expensive pure computations - ✅ Batch API calls - ✅ Use `Map`/`Set` for O(1) lookup - ✅ Virtualize long lists (don't render 10,000 rows) - ✅ Debounce / throttle event handlers - ✅ Use `requestAnimationFrame` for animations - ✅ Lazy-load what you don't need ### Don't premature-optimize "Make it work, make it right, make it fast — in that order." --- ## 12. Language-Specific Notes ### TypeScript / JavaScript - Use `const` by default. `let` only when reassignment is needed. Never `var`. - Use arrow functions for inline, named functions for declarations. - Prefer `===` over `==`. - Use template literals over concatenation. - Use destructuring for object/array access. - Use optional chaining and nullish coalescing (`??`) appropriately. - Don't use `for...in` for arrays. - Don't use `arguments` — use rest parameters. - Use `Map`/`Set` over plain objects/arrays when you need key-based lookup. - Use `URL` and `URLSearchParams` for URL parsing. ### Python - Use type hints (`def parse_invoice(raw: str) -> Invoice: ...`) - Use f-strings, not `%` or `.format()` - Use `pathlib`, not `os.path` - Use dataclasses for value objects - Use `with` for resource management - Don't use mutable default arguments - Don't use `global` (almost never) - List comprehensions are good. Nested ones are not. ### Go - Errors are values: `if err != nil { return err }` - Don't use `panic` for normal flow - Don't use `_` to discard errors (except in defer) - Use `context.Context` for cancellation - Use `gofmt` (no debate) - Use meaningful package names (singular, descriptive) ### React (specific) - Components are functions, named exports, PascalCase - One component per file (mostly — small sub-components can co-locate) - Props are typed with `type`, not `interface` - Don't `useEffect` for derived state — compute it during render - Don't fetch in `useEffect` without a state machine - Memoize when measured, not by default --- ## 13. Code Review Checklist (Before Submitting) For every PR / every function: ### Names - [ ] Names are specific (not `data`, `result`, `item`) - [ ] Functions are verb phrases - [ ] Classes are nouns that mean something - [ ] No boolean flags that change behavior - [ ] No magic numbers — they have names ### Functions - [ ] Each function does one thing - [ ] Each function is < 50 lines - [ ] Each function takes < 4 parameters (or 1 options object) - [ ] No nested conditionals > 3 levels deep - [ ] Early returns for the negative cases - [ ] Pure functions preferred, side effects isolated ### Comments - [ ] Comments explain WHY, not WHAT - [ ] No "this function does X" comments - [ ] No "step 1, step 2, step 3" comments - [ ] TODOs have context (issue link, expected fix) ### Errors - [ ] Errors are handled, not swallowed - [ ] Specific catch types, not generic - [ ] User-facing errors are friendly, internal errors are detailed - [ ] Validation at boundaries ### Types - [ ] No `any` (use `unknown` and narrow) - [ ] No `as any`, no `@ts-ignore` without justification - [ ] Types match reality (no false `as`) ### Tests - [ ] Tests cover behavior, not implementation - [ ] Test names describe what should happen - [ ] Edge cases tested (empty, null, boundary) - [ ] Tests independent of each other ### Structure - [ ] Files < 500 lines - [ ] One responsibility per file - [ ] Imports organized (stdlib, third-party, internal) - [ ] No dead code, no commented-out code ### Style - [ ] Consistent with the rest of the codebase - [ ] Linted and formatted - [ ] No AI-slop patterns from §3 --- ## 14. The Mantra > **Code is read more than it's written. Write for the reader, not the writer.** The next person to read your code is you, six months from now, at 2 AM, debugging a production issue. Be kind to them. Be kind to yourself. > **The best code is the code you deleted.** Every line you didn't write is a line that can't have a bug, can't be misunderstood, can't go stale. > **If the code is good, you won't notice the code. If it's bad, you notice immediately.** Your job is the first. Slop is the second.