Clean-code guidance splits comments into a small set of justified categories and a longer list of harmful ones. Name several of each and state the single criterion that decides which side a comment falls on.
answer
- good: legal, intent, clarification, warning, TODO, amplification, API docs
- bad: redundant, journal, commented-out, noise, banner, attribution, misleading
- criterion: not derivable from code AND not owned by a tool
- VCS owns history, blame, deleted code
- stale comment is worse than no comment
basics
~20 sGood: legal/licence headers, intent, clarification of code you can't change, warnings of consequences, TODOs, and public API docs. Bad: redundant restatement, journal/changelog entries, commented-out code, noise, banners, attributions. Criterion: does it tell the reader something the code cannot?
solid answer
~50 s**Justified comments:** legal or licence headers required by policy; **intent** — why this approach was chosen; **clarification** — translating an opaque value or a third-party API you cannot rename; **warning of consequences** — "not thread-safe", "this test takes 20 minutes"; **TODO/FIXME** with a tracked reference; **amplification** — flagging that something trivial-looking is actually critical; and **public API documentation** for callers who should not read the body. **Harmful comments:** redundant restatement of the code; **journal comments** (a changelog at the top of the file) — version control owns that; **commented-out code** — dead, untested, and unsearchable; noise (`// default constructor`); banner/position markers and closing-brace labels; attributions (`// added by Dave`) — the blame log is authoritative; misleading or stale comments; and comments that exist only to excuse bad naming. The criterion: a comment earns its place only if it carries information the code cannot express and a tool does not already track. Everything else is cost with no benefit.
go deeper
List a few from each side — intent/warning/TODO as good, redundant/commented-out as bad — and say why version control replaces journals and dead code.
Give both lists with examples and state the deciding criterion: information the code cannot express and no tool already tracks.
Add the economics (every comment is unverified, permanently maintained text), the exceptions (legal headers, generated-file warnings, public API docs), and how review enforces it.
Talk about policy: lint rules for TODO references, banning commented-out code in the main branch, deciding which knowledge belongs in ADRs versus inline, and avoiding comment mandates that manufacture rot.
## The framing Comments are not free. Each one is a line that must be read, reviewed, and updated forever, and that nothing verifies. So the useful mental model is a **budget**: a comment must buy its keep by supplying information that (a) the code cannot express and (b) no tool already records. Classic clean-code guidance turns that into two lists. ## Comments that earn their place | Category | What it is | Example | |---|---|---| | **Legal / licence** | Copyright, licence header, compliance text mandated by the organisation. Not for readers — for lawyers and tooling. | `// Copyright (c) ACME. Licensed under Apache-2.0.` | | **Intent / decision** | Why this approach rather than the obvious one. | `// Single-pass scan; the profiler showed the two-pass version dominating p99.` | | **Clarification** | Translating something you cannot rename or restructure — a magic value from a spec, a third-party API's odd contract, a regex. | `// assertEquals(-1, a.compareTo(b)) => a sorts before b` | | **Warning of consequences** | Protecting the next person from a trap. | `// Not thread-safe: caller must hold the session lock.` / `// Do not enable in CI: takes ~20 minutes.` | | **TODO / FIXME** | Known incompleteness, with an owner or issue link so it is trackable. | `// TODO(#4412): remove once v1 clients are retired (est. Q3).` | | **Amplification** | Insisting that something that looks trivial is not. | `// The trim() is load-bearing: leading whitespace breaks the signature check.` | | **Public API documentation** | Contract for callers: parameters, return, errors, side effects, thread-safety. Often tool-generated and published. | Javadoc / docstring / XML doc on an exported function. | Note that most of these are *not* about the mechanics of the code; they are about **context around** the code — history, external constraints, hazards, obligations. ## Comments that cost more than they give - **Redundant** — restates the line beneath it. Zero information; rots on the first edit. - **Journal / changelog comments** — a running history at the top of the file (`2019-04-02 JB: added null check`). This is precisely what version control stores, with author, timestamp, diff and message, searchable and never wrong. In-file journals are duplicated, unverified, and grow forever. - **Commented-out code** — dead code that no compiler checks, no test covers, and no search finds meaningfully. Readers do not dare delete it because they assume it is there for a reason. Version control already holds every deleted line; recover it from history instead. - **Noise comments** — `// default constructor`, `// private field`. Ritual with no content. - **Banners, position markers, closing-brace labels** — `//////// SECTION ////////`, `} // end for`. Occasionally a banner helps in a very long file, but the real signal is that the file or function should be split. - **Attributions and bylines** — `// added by Dave, 2021`. The blame log is authoritative and stays correct through moves and refactors; the comment does not, and it discourages others from touching "Dave's" code. - **Misleading / stale comments** — the worst category, because they cost the reader *more* than silence: they actively cause wrong decisions. - **Mandated comments** — a policy that every function and every field must have a header produces walls of generated-looking prose that nobody reads and nobody updates. - **Apology comments** — `// this is horrible, sorry`. Either fix it or turn it into a tracked TODO. - **Comments compensating for bad names** — the fix is the rename, not the annotation. ## The single criterion, stated precisely > Keep a comment when it conveys information that is **not derivable from the code** and **not already owned by a tool** (version control, issue tracker, blame, tests, types). Otherwise remove it — and if removing it makes the code unclear, change the *code*, not the comment. That one rule generates both lists. Journal comments and attributions fail the "owned by a tool" half. Redundant, noise, banner and closing-brace comments fail the "not derivable" half. Intent, warnings, legal text and TODOs pass both. ## Trade-offs and edge cases - **TODOs are conditionally good.** A TODO with an owner/issue and a removal condition is a lightweight backlog marker. An anonymous TODO from four years ago is just noise; teams often gate this by failing CI on TODOs without an issue reference, or by sweeping them periodically. - **Commented-out code during active debugging** is fine on your own machine; it just must not reach the main branch. - **Legal headers** are pure ceremony from a reader's perspective yet non-negotiable — a reminder that not all comment audiences are humans reading for comprehension. Tools can enforce and generate them. - **Generated files** may carry `// AUTO-GENERATED — DO NOT EDIT`, which is a warning comment and one of the highest-value comments in a repository. - **Culture over rules.** The lists are heuristics; the durable practice is that reviewers treat comments as first-class code, questioning both missing rationale and unearned noise.
- Why are journal comments and author attributions specifically called out, given they do contain real information?Because version control already owns that information and owns it better: commit history and blame record author, date, message and exact diff, stay correct through renames and refactors, and are searchable. The in-file copy is a duplicate that silently drifts and grows without bound.
- How can a team keep TODO comments in the 'good' column at scale?Require a reference in the marker — an issue ID or owner and a removal condition — and enforce it in review or a lint rule. Then sweep periodically: a TODO whose issue is closed, or that no one can justify, gets deleted. Untracked, anonymous TODOs decay into noise.
saying these in an interview costs you the question
- "Keep the old implementation commented out in case we need it" — version control already has it, and the dead copy misleads readers.
- "Every method must have a header comment" — mandated comments produce ritual prose that rots.
- Putting a changelog at the top of the file alongside version control.
- Signing code with `// added by <name>` instead of relying on blame.
- Treating all comments as equally good, so review never pushes back on noise.