ai-superpower/.ai/instructions/skills/clean-code.instructions.md

Clean Code

Based on Robert C. Martin's Clean Code. Apply these principles to all code and configuration, including Helm charts, YAML, and infrastructure-as-code.

---

Naming

• Use intention-revealing names: daysSinceLastPayment not d
• Don't lie with names: accounts not accountList if it isn't a list
• Pronounceable, searchable names — avoid encodings like a2dp, strName
• One word per concept across the codebase: pick fetch, get, or retrieve — not all three
• Names should read like prose: if user.isEligibleForRefund() not if user.check()

Functions

• Do one thing — if you can extract a meaningful function from it, it's doing two things
• One level of abstraction per function — don't mix high-level orchestration with low-level detail in the same function
• Keep short — aim for under 20 lines; use extract method aggressively
• 0–2 arguments preferred; use a parameter object when more are needed
• No side effects — a function named checkPassword must not also start a session
• No flag arguments: delete(file, true) means the function already does two things — split it
• Guard clauses — use early return for validation/error cases at the top of a function to avoid nesting the main flow in an else block. When both branches share code after the split, use if/else instead — it reads more clearly and avoids duplication

Newspaper Readability

Code should read like a good newspaper article: the headline (function/class name) tells you everything, the opening paragraph gives the high-level story, and the details follow below in order. A reader should understand the whole without jumping around.

• High-level first — the top-level function appears at the top of the file; it calls lower-level helpers that follow below it in the order they are called
• One thought per "paragraph" — one function = one clear theme; if you need a heading to separate sections inside a function, extract a function instead
• Consistent vocabulary — use the same terms throughout: pick one word per concept and stick to it
• Smooth flow — control flow reads top-to-bottom; avoid hidden state changes and surprising side effects that break the narrative

Newspaper Test

Ask: "Can someone read this function for 30 seconds and describe what it does without reading any of the helpers it calls?"
If no → the function name or its level of abstraction is wrong.

Single Level of Abstraction (SLAB)

One function = one technical level. High-level orchestration and low-level mechanics must not appear in the same function.

Levels of abstraction (examples):

• High: business flow — processOrder, setupProject, handleUpdateMode
• Mid: coordination — applyDiscount, resolveDocsDir
• Low: mechanics — saveToDB, readFile, arithmetic, string manipulation

Bad — mixed levels:

function processOrder(order: Order): void {
  if (order.total < 100) {                     // high-level policy
    const discount = order.total * 0.1;        // low-level calc
    order.total -= discount;                   // mutation
    db.save('orders', order);                  // external call
  }
}

Good — single level:

function processOrder(order: Order): void {
  const validated  = validateOrder(order);
  const discounted = applyDiscount(validated);
  saveOrder(discounted);
}
// Each helper owns its own level of detail

AI prompts to enforce SLAB:

• "Refactor this function to a single level of abstraction — high-level calls only, extract all low-level logic."
• "Review this code: does each function stay at one abstraction level? Flag any that mix levels."
• "Generate [feature] newspaper-style: headline function first, detail functions below in call order."

Declarative and Imperative Layers

Well-structured code has a clear layer boundary between what and how.

• Top layer — declarative: reads like a description of intent. No loops, no conditionals on mechanics, no string manipulation. A non-programmer should be able to read it and understand the flow.
• Middle layer — still mostly declarative: coordinates steps and error paths, calls lower helpers. May have simple conditionals about what to do, not how.
• Bottom layer — imperative: the actual mechanics. grep, awk, find, SQL, arithmetic, string parsing. This is the only layer where implementation details belong.

// Top — declarative (what happens)
async function runCli(args: CliArgs): Promise<void> {
  const config  = await loadConfig();
  if (args.updateOnly) {
    await handleUpdateMode(args.prevCommit);
  } else {
    await scanProjects(config.devRoot);
    printSummary();
  }
  writeVersionFile(config.devRoot);
}

// Middle — declarative (what each project needs)
async function setupProject(project: Project): Promise<void> {
  ensureSymlink(project);
  ensureGitignore(project);
  const docsDir = await resolveDocsDir(project);
  if (docsDir) await ensureDocFiles(docsDir);
}

// Bottom — imperative (how a symlink is actually created)
function ensureSymlink(project: Project): void {
  const linkPath = path.join(project.path, '.ai');
  if (fs.existsSync(linkPath) && fs.readlinkSync(linkPath) === AI_TARGET) return;
  fs.symlinkSync(AI_TARGET, linkPath);
}

Test: Can you cover the bottom-layer functions and still understand the program from reading only the top two layers? If yes, the layering is correct.

Classes

• Single Responsibility Principle — one reason to change
• Small and cohesive — methods should use the class's fields, not act as utilities
• Hide internals: public API simple, private implementation complex
• No god classes — prefer InvoiceCalculator over BusinessLogic

Error Handling

• Throw exceptions, don't return error codes — let callers handle what they understand
• Descriptive messages: throw new UnauthorizedAccess("Admin role required for this action")
• Define custom exceptions close to where they're used
• Clean up resources: use try/finally or RAII patterns; never leak

Comments

The default is no comments. If you feel the urge to write one, first try to eliminate the need by renaming or extracting a function.

A comment is only justified when the code cannot speak for itself — meaning the why is non-obvious and cannot be expressed in the language itself:

Allowed:

• A bug or quirk in a dependency forces unusual code: # stripe returns 402 for both expired cards and fraud — we treat both as declined
• Non-obvious language or platform behaviour: # BASH_SOURCE[0] is empty when piped through bash
• Timing or ordering constraint that isn't visible from the code: # capture before exec replaces the process
• A trap for the next developer about a hidden coupling: # sets got_new=1 via dynamic scope — caller must declare 'local got_new'

Forbidden:

• Describing what the function name already says: # load docs folders above load_docs_folders()
• Section dividers that replace missing structure: # ── Config ──────────
• Repeating the parameter contract in prose: # Sets RESOLVED_DOCS_DIR; returns 1 if user declines
• Any comment that answers "what" instead of "why"
• TODO comments left in merged code
• Commented-out code

The test: Cover the function name and read only the comment. If someone could have written that comment without reading the code, it adds no value — delete it.

Formatting

• Related code stays close — declarations near first use
• Consistent indentation throughout the codebase — agree once, automate with linting
• No deep nesting — if you're 4 levels deep, extract a function
• Team standard beats personal preference every time
• Always use braces for if, for, while — even for single-line bodies:

  // Bad
  if (docsDir) await ensureDocFiles(docsDir);
  
  // Good
  if (docsDir) {
    await ensureDocFiles(docsDir);
  }
  

  Single-line bodies invite accidents when a second line is added later.
• Avoid optical illusions — code that looks like it does one thing but does another is the most dangerous kind of bug. Common sources:
  • Side effects hidden in expressions: if (user = getUser()) or list.sort() that returns void
  • Boolean logic that reads as simpler than it is: !a || !b — write !(a && b) or extract isInvalid(a, b)
  • Chained calls hiding control flow: items.filter(...).map(...).forEach(save) — if save throws, the chain silently stops
  • Formatting that suggests grouping: unbraced if followed by two lines where only the first is conditional
  • Names that imply the wrong type or behaviour: getUserList() returning a Promise, isReady that triggers a network call

General Principles

• DRY — every piece of knowledge has one representation; duplication is a maintenance liability
• KISS — the simplest solution that works; complexity must earn its place
• Boy Scout Rule — leave the code cleaner than you found it; small improvements compound
• SOLID — especially SRP (above) and Open-Closed: open for extension, closed for modification
• Boundaries — wrap third-party code in adapters; don't let external APIs bleed through the codebase

Testing

• Tests must be FAST, Independent, Repeatable, Self-Validating
• One concept per test — a test that asserts many things is a test that hides failures
• Keep test code as clean as production code — it will be maintained just as long
• Write code testable from day one; retrofitting testability is expensive

---

In Helm Charts and YAML

The same principles apply to configuration:

• Naming: value keys should be intention-revealing — replicaCount not rc, service.port not p
• Single responsibility: one chart does one thing; don't bundle unrelated services
• No magic values: named values in values.yaml, never hardcoded in templates
• DRY: use template helpers (_helpers.tpl) to avoid repeating label blocks and name patterns
• Comments: only when a value's why is non-obvious — a workaround for a known bug, a constraint imposed by an external system, or a counter-intuitive default that will surprise the next person
• Defaults that make sense: a default of false or "" that always needs overriding is not a useful default