Writing Code Comments
Default to no comments. Comments are an exception mechanism, not a communication mechanism. Code should be understood through accurate names, small functions, clear abstractions, and appropriate types. A comment is justified only when the information cannot reasonably be expressed through code structure.
Before writing a comment, run it through the decision process. If none of the four checks pass, delete the comment and improve the code instead.
CSHARP// ❌ Delete this // Check whether prescription is expired. if (prescription.IsExpired) { ... } // ✅ Just this if (prescription.IsExpired) { ... } // ✅ Comment allowed — encodes a regulatory rule the code can't express // National reimbursement rules require quantities to be rounded down. quantity = Math.Floor(quantity);
Apply these checks in order. Stop at the first one that applies.
- Externally-visible behaviour not obvious from the signature? (public endpoint, public API, interface member, exported JS function) → Write XML docs / JSDoc describing the non-obvious behaviour only. Otherwise, don't document it.
- Does this change make an existing comment wrong? (renamed concept, changed rule, changed permissions, changed behaviour) → Fix or remove it in the same change. Never leave stale comments.
- Does it encode a business rule, regulatory constraint, operational requirement, or workaround that code alone can't express? → One brief comment allowed. Otherwise, rename/extract/restructure instead.
- Would a reviewer reasonably suspect a bug without extra context? → A brief local comment allowed. Otherwise, no comment.
Golden rule: Comments explain why (business rules, regulatory requirements, workarounds, unexpected absences). Comments never explain what or how (control flow, steps, variable meaning, obvious conditions). If the code can say it, the code should say it.
When writing or reviewing code:
Progress:
- [ ] Identify any comment candidates in the diff
- [ ] Run each through the 4-step decision process
- [ ] Delete comments that fail all four checks
- [ ] Fix/remove comments made stale by this change
- [ ] For surviving comments, keep them brief and local (not top-of-method essays)
- [ ] Check XML docs/JSDoc only exist where signature doesn't already say enough
- [ ] Verify private methods have no XML docs (rename/split instead)
- [ ] Leave established exceptions alone (SAT comments, #region markers, SQL migration metadata, elision markers)
Example 1 — Business rule (allowed):
Input: if (prescription.IsExpired) return ValidationResult.Invalid();
Output:
CSHARP// Dispensing is prohibited once the prescription reaches its expiry date. if (prescription.IsExpired) { return ValidationResult.Invalid(); }
Reason: domain rule, not derivable from code structure.
Example 2 — Narration (not allowed):
Input: // Save the patient.\nrepository.Save(patient);
Output: repository.Save(patient);
Reason: restates the code; delete.
Example 3 — XML docs justified: Input: A controller action returning 404/403 depending on access. Output:
CSHARP/// <summary> /// Returns the patient record. /// Returns 404 when no record exists. /// Returns 403 when the caller lacks access. /// </summary> Task<IActionResult> GetPatient(int patientId)
Reason: signature alone doesn't convey error/permission behaviour.
Example 4 — XML docs not justified:
Input: Task<PatientDto> GetPatient(int patientId) with /// Gets a patient.
Output: remove the doc comment.
Reason: signature already says this.
Example 5 — Looks like a bug (allowed):
Input: request.Timeout = Timeout.Infinite;
Output: request.Timeout = Timeout.Infinite; // required for long-running exports
Reason: without context this looks like a mistake.
Example 6 — Workaround (allowed): Input: dedup logic on API results. Output:
CSHARP// Third-party API intermittently returns duplicate records. records = records.DistinctBy(x => x.Id).ToList();
- Prefer renaming, extracting methods, or introducing types over adding an explanatory comment.
- Fix or delete stale comments in the same change that invalidates them.
- Keep comment-cleanup scoped to lines you're already touching; don't scope-creep a diff just to remove unrelated comments.
- Private methods never get XML docs — if one seems to need a summary, it needs refactoring instead.
- Document interface members only when callers genuinely can't infer behaviour — not simply because they're public.
- Respect established exceptions: unit test SAT comments,
#regiongrouping, SQL migration metadata (e.g.-- CCRDEV-12341 ...,-- Idempotency guard), and doc-elision markers (/* options */,// ... supplied by caller ...). These are structural/operational, not explanatory prose — don't remove them.
- "This code is complicated so it needs a comment." — Wrong; complicated code needs better structure.
- "I'll explain the method at the top." — If a summary is required to understand it, the method likely does too much.
- "It's only one line." — One-line narrative comments are the most common source of clutter.
- "XML docs are free." — Writing is cheap; reading and maintaining stale docs is not.
- "The reviewer asked for a comment." — Doesn't automatically justify one; apply the decision process. Often naming or restructuring is the better fix.
When reviewing code:
- Do not request comments merely because code is complex, large, or hard to follow — request better naming, smaller methods, extracted concepts, or simpler control flow instead.
- Do not praise comments simply because they exist.
- Flag stale or narrative comments for removal.
- Recommend comments only for business rules, regulatory constraints, workarounds, non-obvious external behaviour, or code that would otherwise look like a bug.
- The preferred review outcome is always clearer code — not more comments.
Final test: Would a competent developer lose important information if this comment disappeared? No → delete it. Yes → keep it brief and local.