On Jul 13, 2012, at 5:57 AM, Stephen Chenney <schen...@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

_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-dev

Reply via email to