4 ms·

Hard to agree or disagree without real examples. I've worked with people who insist on writing paragraphs of stories as comments on top of some pretty obviously

Hard to agree or disagree without real examples. I've worked with people who insist on writing paragraphs of stories as comments on top of some pretty obviously self-descriptive code. In those cases, the comments were indeed just clutter that would likely soon be out of date anyway. Conversely, places that need huge comments like that usually should just be refactored anyway. It's pretty rare to actually need written comments to explain what's going on when the code is written semantically and thoughtfully.

bqmjjx0kac 2y ago
To be clear, I'm not talking about self-indulgent essays embedded in comments. I'm talking about comments that convey context for humans. Here's a made-up example: // Parses a Foo message [spec]. Returns true iff the parse was successful. // The `out` and `in` pointers must be non-null. // // [spec]: https://foo.example/spec bool parse_foo(foo_t* out, const char* in, size_t in_len) { assert(out); assert(in); // TODO(tracker.example/issue#123) Delete the Foo v1 parser. Nobody will // be sending us Foo v1 message when all the clients are upgraded to Bar // v7.1. // // Starting in Foo v2, messages begin with a two-byte version identifier. // Prior to v2, there was no version tag at all, so we have to make an // educated guess. // // For more context on the Foo project's decision to add a version tag: // https://foo.example/specv2#breaking-change-version-tag uint16_t tag_or_len; if (!consume_u16(&tag_or_len, &in, &in_len)) { return false; } if (tag_or_len == in_len) { return parse_foo_v1(out, in, in_len); } return parse_foo_v2(out, in, in_len); } On that old project, I believe the TL would have rejected each of these comments on the grounds that the code is self-documenting. I find this to be absurd: (1) Function comments are necessary to define a contract with the caller. Without it, callers are just guessing at proper use, which is particularly dangerous in C. Imagine if a caller guessed that the parser would gracefully degrade into a validator when `out` is NULL. (This is a mild example! I'm sure I could come up with an example where the consequence is UB or `rm -rf /`.) (2) The TODO comment links to the issue tracker. There's no universe in which a reader could have found issue#123 purely by reading the code. On the aforementioned project, we were constantly rediscovering issues after wasting hours/days retreading old territory. (3) Regarding the "Starting in Foo v2" comment... OK, fine, maybe this could have been inferred from reading the code. But it eases the reader into what's about to happen while providing further context with a link. On balance, I think this kind of "what" comment is worth including even though it mirrors the code.

bqmjjx0kac 2y ago

To be clear, I'm not talking about self-indulgent essays embedded in comments. I'm talking about comments that convey context for humans. Here's a made-up example: // Parses a Foo message [spec]. Returns true iff the parse was successful. // The `out` and `in` pointers must be non-null. // // [spec]: https://foo.example/spec bool parse_foo(foo_t* out, const char* in, size_t in_len) { assert(out); assert(in); // TODO(tracker.example/issue#123) Delete the Foo v1 parser. Nobody will // be sending us Foo v1 message when all the clients are upgraded to Bar // v7.1. // // Starting in Foo v2, messages begin with a two-byte version identifier. // Prior to v2, there was no version tag at all, so we have to make an // educated guess. // // For more context on the Foo project's decision to add a version tag: // https://foo.example/specv2#breaking-change-version-tag uint16_t tag_or_len; if (!consume_u16(&tag_or_len, &in, &in_len)) { return false; } if (tag_or_len == in_len) { return parse_foo_v1(out, in, in_len); } return parse_foo_v2(out, in, in_len); } On that old project, I believe the TL would have rejected each of these comments on the grounds that the code is self-documenting. I find this to be absurd: (1) Function comments are necessary to define a contract with the caller. Without it, callers are just guessing at proper use, which is particularly dangerous in C. Imagine if a caller guessed that the parser would gracefully degrade into a validator when `out` is NULL. (This is a mild example! I'm sure I could come up with an example where the consequence is UB or `rm -rf /`.) (2) The TODO comment links to the issue tracker. There's no universe in which a reader could have found issue#123 purely by reading the code. On the aforementioned project, we were constantly rediscovering issues after wasting hours/days retreading old territory. (3) Regarding the "Starting in Foo v2" comment... OK, fine, maybe this could have been inferred from reading the code. But it eases the reader into what's about to happen while providing further context with a link. On balance, I think this kind of "what" comment is worth including even though it mirrors the code.