Add the core, contest, storage and format layers
Frequencies, bands, modes, callsigns, grid squares and the country file live in Nonemm.Core. Nonemm.Contests holds the scoring engine and CQ WW and CQ WPX. Nonemm.Storage writes N1MM's DXLOG schema, and Nonemm.Formats writes Cabrillo 3.0 and reads and writes ADIF. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
338
.claude/skills/code-style/SKILL.md
Normal file
338
.claude/skills/code-style/SKILL.md
Normal file
@@ -0,0 +1,338 @@
|
||||
---
|
||||
name: code-style
|
||||
description: Language-neutral code style rules - comments, file and folder layout, module dependencies, constants vs literals, OOP vs FP, errors and async, naming, language-specific notes, tests. Load when writing or reviewing code in any language.
|
||||
---
|
||||
|
||||
# Code style
|
||||
|
||||
Sections 2 to 10 of the project style guide. Section 1 (prose) and section 11
|
||||
(the finishing checklist) are in `CLAUDE.md` and load every session.
|
||||
|
||||
Examples are JS-flavoured pseudocode using one running scenario: a service that
|
||||
serves a product catalog over HTTP, backed by a database and a cache. The rules
|
||||
are language-neutral. Translate the syntax; keep the rule.
|
||||
|
||||
---
|
||||
|
||||
## 2. Comments
|
||||
|
||||
Default: **no comment**. Code that reads clearly needs none. Write one only when
|
||||
one of these is true:
|
||||
|
||||
- The *why* isn't visible from the code (a workaround, a spec quirk, a
|
||||
performance trade-off).
|
||||
- The mechanics are genuinely non-obvious (a subtle index, an ordering
|
||||
requirement).
|
||||
- The signature needs a type or unit the code doesn't state.
|
||||
- It's a `TODO` / `FIXME` with an actual next step.
|
||||
|
||||
Exported symbols should have a one-line doc comment when it tells the reader
|
||||
something the name and signature don't. A should, not a must: don't write
|
||||
`/** Returns the base URL. */` above `getBaseUrl()`.
|
||||
|
||||
Keep comments to one line where possible. Lowercase, informal and short is fine:
|
||||
|
||||
```
|
||||
this.pending = []; // indexed by position in the batch
|
||||
|
||||
// group by host so we send one request per host
|
||||
const perHost = new Map();
|
||||
```
|
||||
|
||||
Never write a comment that narrates your thought process, restates the line
|
||||
below it, or explains a design philosophy. Those belong in a design note under
|
||||
`docs/` or in the PR description, not in the source.
|
||||
|
||||
```
|
||||
// BAD — restates the code
|
||||
// Loop over the users and collect their IDs
|
||||
const ids = users.map(u => u.id);
|
||||
|
||||
// BAD — internal monologue
|
||||
// The user will most likely be scanning this list top to bottom, so what they
|
||||
// really want here is for the newest items to feel immediately reachable, which
|
||||
// means we sort descending and let the eye land on the first row.
|
||||
sortDescending(items, 'createdAt');
|
||||
|
||||
// GOOD — explains a non-obvious constraint
|
||||
// the API returns oldest-first; the UI expects the opposite
|
||||
sortDescending(items, 'createdAt');
|
||||
```
|
||||
|
||||
Doc comments only where they add something the declaration doesn't. In a typed
|
||||
language the signature already states most of it, so delete any parameter doc
|
||||
that repeats the type. Keep one like this — the type `string` doesn't say the
|
||||
value is a root URL, and the example shows the expected format:
|
||||
|
||||
```
|
||||
/**
|
||||
* @param baseUrl the site root, e.g. https://example.com/
|
||||
*/
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 3. Files and folders
|
||||
|
||||
One folder per concern. Extensions vary by language; the shape doesn't:
|
||||
|
||||
```
|
||||
src/
|
||||
main entry point, process setup
|
||||
http/
|
||||
index creates the server, registers the routes
|
||||
AuthMiddleware
|
||||
catalog/
|
||||
Product
|
||||
ProductRepository
|
||||
db/
|
||||
index
|
||||
util/
|
||||
hashing
|
||||
```
|
||||
|
||||
- **Group folders by subject, not by shape.** `catalog/` holds everything about
|
||||
products. Don't create `helpers/` or `constants/` folders.
|
||||
- **A shared types or declarations folder is for declarations, not domain
|
||||
types.** Ambient declarations, generated API clients and types used across
|
||||
several folders belong there. A type used by one module stays in that module.
|
||||
- **A module's entry point file is an entry point, not a barrel.** It creates
|
||||
what the module provides and exports the function the caller calls. Don't fill
|
||||
it with re-exports of the folder's internals.
|
||||
- **One class per file, named after the class.** Small related types can share a
|
||||
file, but the file name must match what's inside.
|
||||
- **Free functions go in a lowercase-named file** describing the topic
|
||||
(`hashing`, `formatting`), not a catch-all `utils`.
|
||||
- **Keep files small.** Over ~300 lines, check whether part of the file is a
|
||||
separate concern and split it out.
|
||||
- **Don't put a class, unrelated helper functions and a constants block in the
|
||||
same file.** Split them into three files.
|
||||
- **Design notes go in `docs/`.** Anything explaining why the system is shaped
|
||||
the way it is — decisions, trade-offs, rejected alternatives — goes in a file
|
||||
under `docs/`, not in a source comment.
|
||||
|
||||
If a piece of code is only ever used in relation to one class or module, put it
|
||||
there. `Product.getImageUrl()` is a method on `Product`, not a
|
||||
`buildProductImageUrl(product)` in `util/`. Don't make a file per method.
|
||||
|
||||
Move code into a shared module when it gets a second caller, not before.
|
||||
|
||||
---
|
||||
|
||||
## 4. Dependencies between modules
|
||||
|
||||
Dependencies point in one direction: entry point → wiring → workers → the
|
||||
modules at the end of the chain.
|
||||
|
||||
```
|
||||
main → http/index → catalog/ProductRepository → util/hashing
|
||||
→ catalog/Product
|
||||
→ db/index
|
||||
```
|
||||
|
||||
- **No cycles.** If module A imports B and B imports A, fix it one of two ways:
|
||||
move the shared code into a third module both can import, or move the method
|
||||
into the module where the data it uses is defined.
|
||||
- **Give the modules at the end of the chain no project imports.** `hashing` and
|
||||
`Product` import only third-party packages, so you can move them or test them
|
||||
on their own.
|
||||
- **Pass dependencies in as constructor arguments.** `ProductRepository` takes
|
||||
the database and cache clients in its constructor; `AuthMiddleware` takes a
|
||||
`verifyToken` callback. Neither imports the config or a global singleton.
|
||||
|
||||
```
|
||||
// BAD — reaches for a module-level singleton and the global config
|
||||
class ProductRepository {
|
||||
find(id) {
|
||||
return cache.get(config.keyPrefix + id); // both imported at the top
|
||||
}
|
||||
}
|
||||
|
||||
// GOOD — dependencies passed in by the caller
|
||||
class ProductRepository {
|
||||
constructor(cache, keyPrefix) { ... }
|
||||
|
||||
find(id) {
|
||||
return this.cache.get(this.keyPrefix + id);
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
- **Import the config only in the entry point and the wiring files.** Pass the
|
||||
values down as arguments. No other module reads it.
|
||||
- **Declare each value and each rule in one file.** The list of upstream hosts is
|
||||
in `http/index` and nowhere else. If the same literal or the same logic appears
|
||||
in two files, delete one and import it from the other.
|
||||
|
||||
Single responsibility, practically. Ask: *can I describe this file's job in one
|
||||
sentence, without "and"?*
|
||||
|
||||
- `Product`: holds the fields of one catalog item.
|
||||
- `ProductRepository`: loads products from the database.
|
||||
- `AuthMiddleware`: checks the Authorization header.
|
||||
- `hashing`: turns an object into a stable hash.
|
||||
|
||||
Not everything needs its own module. A helper called from one place can stay a
|
||||
local function in that file. Split a file when it covers two subjects that don't
|
||||
share state.
|
||||
|
||||
---
|
||||
|
||||
## 5. Constants vs literals
|
||||
|
||||
Extract a value when it is **repeated**, **configurable**, or **not
|
||||
understandable from the value alone**. Otherwise inline it.
|
||||
|
||||
```
|
||||
// BAD
|
||||
const METRES_PER_KILOMETRE = 1000;
|
||||
const CACHE_KEY_PREFIX = 'product:';
|
||||
|
||||
// GOOD — used once, obvious in place
|
||||
const km = metres / 1000;
|
||||
const key = 'product:' + hashObject(query);
|
||||
```
|
||||
|
||||
- Unit conversions, `0`, `1`, `-1`, `100` for percent: inline.
|
||||
- A magic number whose meaning isn't obvious (`0o666`, `86_400`, a retry limit
|
||||
someone will want to tune): name it, or add a short comment.
|
||||
- A value used in three places: declare it once in the module that uses it most,
|
||||
and import it from there. Not in a global constants file.
|
||||
- A value that must stay in sync with something outside the file (a server limit,
|
||||
a CSS breakpoint): name it, and say in a comment what it matches.
|
||||
- Anything deployment-specific (URLs, ports, keys, limits): config file.
|
||||
|
||||
---
|
||||
|
||||
## 6. OOP and FP
|
||||
|
||||
Applies in every language that has both. Pick one per module and stay with it.
|
||||
|
||||
- **Class** when there is state that persists between calls, or several methods
|
||||
over the same data: a queue that holds pending items, a client that holds a
|
||||
connection.
|
||||
- **Plain function** when it's a transform with no state: `hashObject`,
|
||||
`formatPrice`.
|
||||
- **Don't mix them in one file.** Move exported free functions out of a file that
|
||||
defines a class.
|
||||
- **No class with only static methods.** Export the functions from a module, or
|
||||
use whatever the language offers for free functions.
|
||||
- **No inheritance unless the subclass is a kind of the base class.** Otherwise
|
||||
pass a function or an object in, as `AuthMiddleware` takes `verifyToken`.
|
||||
|
||||
### State and mutation
|
||||
|
||||
- Don't mutate function arguments. Return a new value instead.
|
||||
- Don't export mutable module-level or global state. If state is shared, put it
|
||||
in a class and pass the instance to whoever needs it.
|
||||
- Know which of your language's collection operations mutate in place and which
|
||||
return a copy. Sorting and reversing usually mutate: `sort()`, `reverse()`,
|
||||
`splice()` and `push()` in JS, `list.sort()` and `list.append()` in Python,
|
||||
`std::sort` in C++, `Collections.sort` in Java. Copy first when the caller
|
||||
still needs the original.
|
||||
- Watch for shared references when creating collections. Filling n slots with one
|
||||
empty collection puts the *same* object in every slot, so writing to one writes
|
||||
to all. This bites in JS (`new Array(n).fill([])`), Python (`[[]] * n`) and
|
||||
anywhere else that copies the reference rather than the value. Construct each
|
||||
element separately.
|
||||
|
||||
---
|
||||
|
||||
## 7. Errors and async
|
||||
|
||||
- **Don't swallow errors.** A `catch` block either handles the failure (retry,
|
||||
documented fallback) or rethrows. Logging and continuing is not handling it.
|
||||
- **The caller must be able to tell a failure from an empty result.** Don't
|
||||
return an empty list, null or nothing for both. Throw, or return a result type
|
||||
that states which happened.
|
||||
- **Catch around the statement that can fail**, not around the whole function
|
||||
body.
|
||||
- **Put the values needed to debug in the message**: the URL, the id, the status
|
||||
code. Not `'Request failed'`.
|
||||
- **Rethrow with added context or not at all.** `catch (e) { throw e; }` is
|
||||
noise. Preserve the original error as the cause where the language supports it.
|
||||
- **No unobserved async results.** Every promise, future or task is awaited,
|
||||
returned, or has an error handler attached. Turn on the linter rule for this if
|
||||
the language has one.
|
||||
- **Don't mix callbacks and promises** in one code path.
|
||||
|
||||
```
|
||||
// BAD — the caller gets nothing back and cannot tell why
|
||||
async function fetchProducts(url) {
|
||||
try {
|
||||
return await post(url, body);
|
||||
} catch (e) {
|
||||
console.error('fetch rejected', e);
|
||||
}
|
||||
}
|
||||
|
||||
// GOOD — the error propagates, with the context to debug it
|
||||
async function fetchProducts(url) {
|
||||
try {
|
||||
return await post(url, body);
|
||||
} catch (cause) {
|
||||
throw new Error(`product request to ${url} failed`, { cause });
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 8. Naming
|
||||
|
||||
Follow the dominant convention of the language you're in: `PascalCase` classes
|
||||
with `camelCase` members in TS, Java and C#; `snake_case` in Python, Rust and C.
|
||||
Don't import another language's casing. On top of that:
|
||||
|
||||
- No Hungarian notation and no `I` prefix on interfaces.
|
||||
- Constant casing only for values that never change.
|
||||
- File names match the main export: the class name for a class, a topic name for
|
||||
a group of functions.
|
||||
- Functions start with a verb: `buildUrl()`, not `urlBuilder()`. Booleans start
|
||||
with `is`, `has` or `should`.
|
||||
- Don't repeat the containing type in a member name: `Product.getImageUrl()`, not
|
||||
`Product.getProductImageUrl()`.
|
||||
- No invented abbreviations. `id`, `url`, `db`, `req`/`res` are fine because
|
||||
everyone reads them; `prdct` or `rqSndr` are not.
|
||||
|
||||
---
|
||||
|
||||
## 9. Language-specific notes
|
||||
|
||||
Replace this section with the rules for the language in use. TypeScript, as a
|
||||
worked example:
|
||||
|
||||
- No `any`. Use `unknown` at boundaries and narrow.
|
||||
- Define types in the module that uses them. A shared `types.ts` only for types
|
||||
genuinely used across several folders.
|
||||
- Prefer `type` for unions and object shapes, `interface` for things meant to be
|
||||
implemented.
|
||||
- Don't add a wrapper type until it prevents a real mistake. `string` is fine for
|
||||
a URL until you have two kinds of string that could be swapped.
|
||||
- `strict: true` in `tsconfig.json`. Don't disable a check for one file.
|
||||
- No `as` casts to force a shape. Parse external data into a typed value at the
|
||||
boundary and let the check fail there.
|
||||
- No non-null assertion (`!`) without a comment saying why the value can't be
|
||||
null.
|
||||
- `readonly` on fields and parameters that are never reassigned; `as const` for
|
||||
literal tables.
|
||||
- Exhaustive `switch` on a union: add a `default` that assigns to `never`, so
|
||||
adding a case breaks the build.
|
||||
|
||||
The equivalents elsewhere: `const` correctness and RAII in C++, `final` and
|
||||
`Optional` in Java, strict types and typed properties in PHP, type hints checked
|
||||
by a type checker in Python.
|
||||
|
||||
---
|
||||
|
||||
## 10. Tests
|
||||
|
||||
- Test through the module's entry point, not its internals. A test that calls a
|
||||
private method breaks on every refactor.
|
||||
- One behavior per test, named after what it asserts.
|
||||
- No mocking framework for our own code. Pass a fake in through the constructor.
|
||||
That is what section 4 is for.
|
||||
- Test files sit next to the code they test, following the language's convention
|
||||
for test file names.
|
||||
- A bug fix includes the test that reproduces the bug.
|
||||
- Don't test getters, types, or that a library works.
|
||||
Reference in New Issue
Block a user