Files
skills/skills/clean-code/SKILL.md
T
mathias 1d140ec525
CI / Check / Test (push) Successful in 1s
CI / Tag release (push) Successful in 1s
feat(check): every skill states when it stops and hands back
Implements #5's decision. The hard part was #37's, not #5's: which skills need
an escalation rule? A hardcoded list rots, sniffing the prose for "does this
write anything" is unfalsifiable, and a frontmatter flag is the option #5
rejected wearing a hat.

So there is no acting set. Every skill answers the question, and a read-only one
answers it in a line. That turns an absence into a claim someone can disagree
with, which is the whole point — "this skill never hands back" is a design
decision worth stating, and today it was indistinguishable from nobody having
thought about it.

Eleven written here, all advisory or read-only, where the honest answer is short
and the value is in being specific rather than generic. `cognitive-load` stops
when the metric disagrees with the people reading the code, because that is
evidence about the metric. `test-design` stops when the tests are fine and the
design under test is what makes them awkward. `code-review` stops when the diff
is big enough that a review would be a skim, and says so rather than producing
confident coverage of a fraction.

The remaining thirty are the acting set — writes, dispatches, credentials,
releases — and each needs a rule someone thought about rather than a rote
paragraph, so they sit in bin/escalation-pending.txt. That list only shrinks,
and it is self-cleaning: a listed skill that HAS a section fails the gate, so an
exemption cannot outlive its reason. Mutation-verified both ways.

Six test cases including the stale-entry case, because an exemption for a skill
that no longer exists is how a gate quietly stops covering something.

Refs #37, closes the mechanism half
Bump-Type: minor
2026-08-29 14:56:04 +02:00

9.2 KiB

name, surfaces, summary, use_when, description
name surfaces summary use_when description
clean-code
claude-code
library
Clean code principles with Go adaptation Code review, refactoring, new code Write code that humans can read, understand, and safely change. Apply SOLID, GRASP, and Clean Code principles during the REFACTOR phase of TDD.

Clean Code

Core Philosophy

Write code for humans first, computers second. Every piece of code you write will be read many more times than it is written. Optimize for clarity and simplicity above all else.

Code has three consumers:

  1. Users — get their needs met
  2. Customers — make or save money
  3. Developers — must maintain it

Developers read code 10x more than they write it. Design for them.

When to Apply This Skill

  • During the REFACTOR phase of TDD (after the test is green)
  • When reviewing code (load code-review skill too)
  • When designing new types or packages

Do not apply during the GREEN phase. Ugly code that passes is correct. Clean it after.

Naming

Naming is the most important thing in code. In priority order:

  1. Consistency — Same concept = same name everywhere. One name per concept.
  2. Understandability — Domain language, not technical jargon.
  3. Specificity — Precise, not vague. Avoid data, info, manager, handler, processor, utils.
  4. Brevity — Short but not cryptic.
  5. Searchability — Unique, greppable names.
  6. Pronounceability — You should be able to say it in conversation.
// Bad: inconsistent, vague, cryptic
func getUsrById(id string) {}
func fetchCustomerByID(id string) {}
func retrieveClientById(id string) {}

// Good: consistent, specific
func GetUser(ctx context.Context, id UserID) (User, error) {}
func GetOrder(ctx context.Context, id OrderID) (Order, error) {}

Functions/Methods

  • Do one thing, do it well, do it only
  • Operate at a single level of abstraction
  • Prefer fewer parameters — zero is ideal, one or two common, three should be rare
  • Avoid side effects — if a function has a side effect, name it accordingly
  • Prefer pure functions where possible
// Bad: does too much, mixed abstraction levels
func processOrder(o *Order, db *sql.DB, mailer Mailer) error {
    if o.Items == nil || len(o.Items) == 0 {
        return errors.New("empty order")
    }
    total := 0.0
    for _, item := range o.Items {
        total += item.Price * float64(item.Qty)
    }
    if _, err := db.Exec("INSERT INTO orders ...", o.ID, total); err != nil {
        return fmt.Errorf("save order: %w", err)
    }
    if err := mailer.Send(o.Customer.Email, "confirmed"); err != nil {
        return fmt.Errorf("send confirmation: %w", err)
    }
    return nil
}

// Good: single level of abstraction, composed from focused functions
func ProcessOrder(ctx context.Context, o *Order, repo OrderRepository, mailer Mailer) error {
    if err := validateOrder(o); err != nil {
        return fmt.Errorf("validate: %w", err)
    }
    total := calculateTotal(o.Items)
    if err := repo.Save(ctx, o, total); err != nil {
        return fmt.Errorf("save: %w", err)
    }
    if err := mailer.SendConfirmation(ctx, o.Customer.Email); err != nil {
        return fmt.Errorf("notify: %w", err)
    }
    return nil
}

Error Handling (Go-Specific)

In Go, error returns are a first-class feature, not a smell. Follow these rules:

// Always wrap with context
if err != nil {
    return fmt.Errorf("parse config: %w", err)
}

// Never: naked return without context
if err != nil {
    return err
}

// Never: log and return (pick one)
if err != nil {
    log.Error("failed", "err", err)
    return err // caller gets logged error AND returns it — double-counted
}

// Never: swallow errors silently
_ = doSomething()

Error wrapping builds a call-chain narrative: "http handler: parse request: validate email: invalid format". This is valuable, not verbose.

Comments

The best comment is code that doesn't need one. Before writing a comment, ask whether you could make the code itself clearer.

When comments are necessary, explain why, not what. The code shows what happens; comments explain decisions and intent.

// Bad: explains what (redundant)
// increment counter
i++

// Good: explains why
// Offset by 1 because the upstream API uses 1-based page numbering
page := requestedPage + 1

Go adaptation: godoc comments on exported identifiers ARE documentation, not a failure. The "comments are failure" rule applies to inline comments explaining bad code. Exported types and functions must have godoc comments.

// UserService handles user lifecycle operations.
// It is safe for concurrent use.
type UserService struct { ... }

// GetByEmail returns the user with the given email address.
// Returns ErrNotFound if no user exists with that email.
func (s *UserService) GetByEmail(ctx context.Context, email Email) (User, error) { ... }

SOLID Principles (Brief)

For full SOLID guidance with Go examples, load the solid skill.

Principle Quick test
SRP "Does this type have ONE reason to change?"
OCP "Can I add behavior by implementing an interface, not editing this code?"
LSP "Can callers substitute any implementation without knowing the difference?"
ISP "Is this interface the smallest it can be?" (io.Reader is a model)
DIP "Do I pass interfaces as parameters, not concrete types?"

GRASP Principles

Information Expert: Assign responsibility to the type that has the data needed to fulfil it.

Low Coupling: Minimize dependencies between packages. When one changes, few others should too.

High Cohesion: Keep related functionality together. Everything in a package should relate to its central purpose.

Controller: One type handles system events for a use case.

Tell, Don't Ask: Tell objects what to do rather than asking for their data and acting on it.

Go-Specific Clean Code Rules

Prefer interfaces over concrete types in function signatures

// Bad: tied to a concrete implementation
func NewHandler(db *postgres.DB) *Handler { ... }

// Good: depends on the behavior needed
func NewHandler(store UserStore) *Handler { ... }

Small, focused interfaces

// The io.Reader/io.Writer pattern is the Go ideal
type Reader interface {
    Read(p []byte) (n int, err error)
}

// If you need both, compose at the call site
type ReadWriter interface {
    Reader
    Writer
}

Embedding over inheritance

// Use embedding for composition, not to inherit behavior you'll override
type LoggedHandler struct {
    Handler
    logger *slog.Logger
}

Constructor injection for dependencies

type Service struct {
    repo   Repository
    mailer Mailer
    logger *slog.Logger
}

func NewService(repo Repository, mailer Mailer, logger *slog.Logger) *Service {
    return &Service{repo: repo, mailer: mailer, logger: logger}
}

Avoid package-level state

Package-level variables are global state. They make tests unreliable and code hard to reason about.

// Bad
var db *sql.DB

// Good: pass as dependency
type Handler struct {
    db *sql.DB
}

Code Smell Detection (Quick Reference)

During the REFACTOR phase, look for these smells and address them:

Smell Go Signal Fix
Long function > ~30 lines (Go can go longer, but be honest) Extract function
Too many parameters > 3-4 parameters Use a config struct or option pattern
Divergent change One type changes for multiple unrelated reasons Split into focused types
Feature envy Method uses another type's data more than its own Move method to that type
Magic numbers Bare literals Named constants or typed config
Deep nesting > 3 levels of indentation Early returns, extract helpers

For comprehensive smell detection, load the code-review skill.

Structural Principles

Separation of concerns: Business logic should know nothing of HTTP, SQL, or filesystems.

Modularity: Each package understandable in isolation.

Command-Query Separation: Functions either do something (command) or return something (query), not both.

Principle of Least Surprise: Code should behave the way readers expect.

Boy Scout Rule: Leave the code better than you found it.

Pre-Refactor Checklist

Before cleaning up code:

  • All tests pass
  • Race detector clean: go test -race ./...
  • I understand what the code does
  • I have a clear target design in mind

During refactoring:

  • Tests stay green after each change
  • Each change is small and focused
  • Commit after each meaningful improvement

References

  • references/clean-code.md — detailed naming rules, object calisthenics
  • references/code-smells.md — comprehensive smell catalog with examples
  • references/python-adaptation.md — Python conventions: error chaining, dataclasses, the truthiness trap
  • Load solid skill for SOLID principles with Go examples
  • Load refactoring skill for systematic refactoring techniques

Escalation

Advisory during the REFACTOR phase. Suggests; does not restructure behaviour.

Stop and hand back when: the cleanup you are proposing changes behaviour. That is not refactoring, it is a change, and it needs its own failing test first — hand back to tdd.