comments · diff
git:20260214.5b4ea24 to git:20260507.7523b16
54 added, 46 removed. Audit A to A.
---
- name: effective-commenting
- description: Best practices for writing meaningful and clean code comments.
+ name: comments
+ description: Decide whether a comment is needed and write a useful one when it is — focused on the "why" behind non-obvious code, not narration of what the code does. Use this skill when adding or editing comments, reviewing a diff for excess commentary, cleaning up "AI thought" trails, writing doc comments for public APIs, or when the user pushes back on noisy/redundant comments.
---
- # Effective Commenting
-
- ## When to use
+ # Comments
- - Explaining complex business logic or algorithms.
- - Documenting public APIs (Data layer → Domain layer).
- - Clarifying non-obvious workarounds, hacks, or side effects.
- - Warning about potential pitfalls or future refactoring needs.
+ Decide whether a comment belongs, then make the comment earn its place.
## Steps
- 1. **Assess Necessity**: Review if the code can explain itself; if so, proceed without comments. Only comment if the code cannot convey the full context.
- 2. **Clarify via Naming**: Rename methods and variables to describe their purpose explicitly.
- 3. **Document Intent**: Write comments that explain the "why" (business context, constraints) behind the implementation.
- 4. **Finalize Output**: Ensure only the active, necessary code remains, removing any temporary artifacts or disabled blocks.
- 5. **Distinguish Scope**: Use `///` for public API documentation and `//` for internal implementation notes.
-
- ## Code
+ 1. **Try removing the comment first.** Read the line(s) without it. If a competent reader still understands the code, leave the comment out.
+ 2. **Rename before commenting.** If the intent is unclear, the fix is usually a sharper identifier (`isExpired` beats `// check if expired`). Reach for a comment only when naming and structure can't carry the meaning.
+ 3. **Identify the _why_.** A comment belongs when it captures something the code itself cannot show:
+ - a hidden constraint or invariant
+ - an external quirk (API behavior, browser bug, ordering requirement)
+ - a workaround for a known issue (name the issue)
+ - a surprise the reader would otherwise question
+ 4. **Write it tight.** One line where possible. State the reason, not the mechanism. Skip phrases like "this function…" or "we will now…".
+ 5. **Public APIs get doc comments.** Document the contract — inputs, outputs, thrown errors, side effects — using the language's doc syntax (`///`, `/** */`, docstrings). Leave internal helpers quiet unless they hide a non-obvious constraint.
+ 6. **Match the codebase.** Before adding comments to a new area, read 5–10 nearby files. Mirror their comment density, tone, and doc-comment style.
- ### Bad Examples (Avoid)
+ ## Anti-pattern: narration & AI thought trails
- ```dart
- // Step 1: Initialize list of users
- // This variable stores user data
- final users = <User>[];
+ ```ts
+ // Step 1: get users
+ const users = await fetchUsers();
- // Loop through users to save them
- for (var u in users) {
- // validation logic
+ // loop through users
+ for (const u of users) {
+ // validate user
if (u.isValid) {
- save(u); // AI thought: saving to database here
+ save(u); // save to db
}
}
- // Code below is deprecated
- // void oldFunction() {
- // print('old logic');
- // }
+ // AI thought: this handles the empty case
+ if (users.length === 0) return;
+
+ // old logic — keep just in case
+ // function oldThing() { ... }
```
- ### Good Examples
+ This narrates what the code already says, leaves planning markers, and keeps dead code "just in case". Strip it.
- ```dart
- /// Syncs user data with remote backend.
- ///
- /// Throws [SyncException] if network fails or data is invalid.
- Future<void> syncUser() async {
- // Use a LinkedHashMap to preserve insertion order for UI rendering,
- // as the backend returns unordered JSON but order matters here.
- final cache = <String, User>{};
+ ## Pattern: doc the contract, capture the surprise
+ ```ts
+ /**
+ * Synchronizes user records with the remote backend.
+ *
+ * Throws SyncError if the network fails or the payload is malformed.
+ */
+ async function syncUsers(): Promise<void> {
+ // Backend returns 404 for "no data yet" on freshly provisioned accounts —
+ // treat it as an empty result, not an error.
try {
- await _api.fetch();
+ await api.fetch();
} catch (e) {
- // Suppress 404 errors as they indicate "no data yet" for this specific
- // endpoint, which is a valid state for new users.
- if (e is NetworkException && e.statusCode == 404) return;
- rethrow;
+ if (isNotFound(e)) return;
+ throw e;
}
}
- /// Example of self-explanatory code (No comments needed)
- bool get isAdult => age >= 18;
-
+ // Self-explanatory — leave it alone.
+ const isAdult = (user: User) => user.age >= 18;
```
+
+ The doc comment states the contract; the inline comment captures a non-obvious upstream quirk; trivial code is left to speak for itself.
+
+ ## Edge cases
+
+ - **Apologies in code** ("// hacky but works") — fix the code or file a tracked issue; let the comment go.
+ - **Task / PR / caller references** ("// added for ticket X", "// used by Y") — keep that context in the commit message; the source of truth shouldn't follow the work item around.
+ - **`TODO` / `FIXME` markers** — pair every one with an owner or a tracked issue, or resolve it now.
+ - **Translating code line-by-line into English (or any language)** — that's narration. Comment a constraint or a surprise instead, or remove the comment entirely.
+ - **Section banners** (`// ===== HELPERS =====`) — match the file: use them when nearby files already do, leave them out when they don't.
+ - **Commented-out code** — delete it. Git keeps history.