clean-code

Files (1)

• SKILL.md

SKILL.md (first lines)

# Clean Code: Structure, Naming and Cohesion

Most defects are not clever. They are the ordinary consequence of code that is harder to read
than it needed to be. A function that does three things hides which one broke. A name that
lies sends the next reader to the wrong file. A module that depends on everything cannot be
changed without changing everything.

So the work is not cleverness. It is applying a small number of well-established structural
disciplines consistently, and being willing to keep applying them after the code already
works.

**The line that does most of the work:**

> Code is read far more often than it is written, and almost always by someone with less
> context than the author had. Optimise for the reader who arrives in six months knowing
> nothing. If your change makes sense only because you remember why, it is not finished.

## The five disciplines

Every recommendation below reduces to one of these. Naming them keeps a review from
collapsing into taste.

**Single responsibility.** A unit should have one reason to change. When you cannot describe
a function without "and", it is doing two things, and the two things will need to change on
different schedules. Split along the seam where the reasons differ, not where the line count
is convenient.

**Cohesion over proximity.** Things that change together belong together. Code grouped by
technical layer (all the controllers here, all the models there) scatters a single feature
across the tree, so every change touches five directories. Group by what the code is about.

**Explicit dependencies.** A unit should declare what it needs rather than reach for it.
Constructor parameters and function arguments are declarations; module-level singletons,
global config and ambient state are not. The test for this is whether the unit can be
exercised without standing up its whole world.

**Names that survive being read alone.** A name is the only documentation that cannot go
stale, because changing the code without changing the name is visible. Prefer a longer name
that is accurate to a short one that is approximately right. `elapsed_ms` beats `t`.

**Duplication is cheaper than the wrong abstraction.** Two similar blocks are a fact. One
premature abstraction over them is a commitment, and unwinding it later costs more than the
duplication ever did. Wait until the third occurrence, and until the three genuinely share a
reason to change rather than a shape.

## How to work

1. **Read before you write.** Understand what the current shape is for. Code that looks wrong
   is often load-bearing in a way the diff does not show.
2. **Name the problem before proposing the fix.** "This function is 200 lines" is an
   observation. "This function mixes request parsing, business rules and persistence, so a
   change to any one of them risks the other two" is a problem.
3. **Prefer the smallest change that removes the problem.** A rewrite that also improves five
   unrelated things cannot be reviewed, so it will be approved on trust rather than reading.
4. **Say what you did not change and why.** A review that only lists changes reads as though
   everything else was examined and approved.
5. **Leave the reasoning, not just the result.** The next reader needs to know which
   constraint drove the shape.

## Routing

Read the sub-skill matching the work, then follow it. If more than one applies, read both; if
none clearly applies, continue with this document.

| Sub-skill | Use for |
|---|---|
| `design` | Designing a new interface, module, schema or type. What the shape should be before it has callers. |
| `audit` | Reviewing code that already exists for structural problems. Diffs, PRs, whole files. |
| `retro` | Something broke and you are deciding what to change so the class of problem is less likely. |
| `ux` | Forms, flows and screens. Structure and clarity of user-facing interaction code. |
| `authz` | Permission and access-control code. Structure, clarity and testability of authorisation logic. |
| `data` | Pipelines, transformations, queries and reporting code. |
| `ops` | Deployment, migration, configuration and infrastructure code. |
| `guardrails` | Lint configuration, CI setup, formatting rules and repository conventions. |
| `agent-guardrails` | Repository configuration for AI coding assistants. |
| `llm` | Code that calls a language model API and handles its output. |

## What good output looks like

Concrete, ordered by impact, and anchored to the code in front of you.

- **Point at specific lines.** "The `process` function" is reviewable. "The code" is not.
- **Order by cost of leaving it.** A misleading name in a widely-called function costs more than a long function nobody touches.
- **Show the shape you mean.** A two-line before-and-after communicates more than a paragraph describing it.
- **Separate the structural from the stylistic.** Formatting is not a finding; a tool should already own it. Spending review attention on brace placement is how the structural comments get skimmed.
- **Do not pad.** Three findings that matter beat eleven that include four restatements of the same point and three matters of preference.

## What to avoid

**Cargo-cult patterns.** A factory that has one implementation, an interface with one
implementer, a layer that only forwards calls. Each adds a hop the reader must follow and
buys nothing until the second case actually exists.

**Rewriting to taste.** If the existing code is consistent and clear but not how you would
have written it, that is not a finding. Consistency within a codebase is worth more than any
individual improvement.

**Advice that is only true in the abstract.** "Reduce coupling" is not actionable. "The
report builder imports the HTTP client directly, so it cannot be tested without a network
stub; pass the fetched rows in instead" is.

**Confusing length with complexity.** A long function that does one thing in a straight line
is easier to follow than three short ones that pass state between them. Count reasons to
change, not lines.

## What it looks like in practice

A worked example, because the disciplines above are easy to agree with and hard to apply.

Here is a function that works, passes its tests, and is still a problem:

```python
def process(data, flag, config):
    if flag:
        rows = [r for r in data if r.get("active")]
    else:
        rows = data
    out = []
    for r in rows:

From rainmanjam/poka-yoke, MIT.

More from rainmanjam/poka-yoke