hermes-hub/.agents/skills/frontend-design/code-style.md

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> — 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<void> {...}

---

7. Error Handling

Errors are values

Treat errors as data, not as control flow exceptions. In TypeScript:

✅ type Result<T> = { 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(<Profile userId="123" />)
  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.