[webkit-dev] Comments in the code (Was Please include function-level comments in change log entries)

[Maciej Stachowiak]

On Jul 13, 2012, at 5:57 AM, Stephen Chenney <schenney at chromium.org> wrote:

> 
> I'd be happy to add a term to the cost function:
>> Cost per year with good comments:  t_maintainComments * n_patches + t_understandWithComment * n_engineersNeedingToUnderstand + t_discoverAndFixBadCommend * n_badComments
> 
> I don't doubt there are poor comments, both outdated and useless. That's a reviewing failure. You have simply highlighted the fact that any standard for comments requires reviewer attention. Hence "cost of maintaining comments".

I agree that reviewers should try to prevent inclusion of low-quality comments. I think the best way to avoid poor comments is for reviewers to be skeptical of every comment they see, and ask if the same information can be expressed as well in the code itself. There may be cases where it can't, typically "why" comments that explain the reason for doing something. But if there are more comment lines than code lines in a function, it probably needs rewriting.

> 
> Thus, I'm much more interested in comments that pass the filter of people who prefer fewer comments and thus would spend their limited comment budget on ones that truly have value, than comments from people who believe in adding lots of comments. My Bayesian inference is that comments from the latter group have much lower average value per comment. When adding a comment, you should really think about whether the value outweighs the cost, just as when adding a line of actual functional code.
> 
> Yes. I don't think you'll find much disagreement. I don't believe anyone is arguing for "lots of comments". The primary focus of this discussion, as I recall reading, is: (1) class and member comments that describe system behavior, (2) comments on invariants in code and (3) references to sections of the spec that define behavior, and where we deviate.

Really? I think there is an implicit assumption on the part of some that "good comments" means many comments, or at least, more than we typically have. For your specific examples, my opinions would be:

(1a) Class comments can be useful if the class cannot be adequately and clearly described by its name. But ideally they should describe local properties of the class, not global properties of the system. They should especially avoid documenting facts that may change without the class holding the comment itself changing.
(1b) Member comments are almost always bad. You will not see them at the point of use, and new uses will typically be produced by copying existing uses, not by reading the header. It is almost always superior to use a better name for the member. If you think you need a member comment, you almost surely gave the member a bad name.
(2) Invariants in the code should be ASSERTs or COMPILE_ASSERTs, not comments.
(3) Spec section references are almost always not worth it. People working on a particular aspect of Web technology should be aware of the relevant specs. And when the spec is updated, thus invalidating all the section numbers, you have a bunch of out-of-date comments.
(3b) Documenting intentional noncompliance with a standard may be useful, but the key is to explain *why* it's necessary to deviate from the standard, not just how we're deviating, otherwise the next person to look at the code won't know whether it's ok to change the code to be compliant.

Regards,
Maciej

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120713/85708ae2/attachment.html>