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

[Alec Flett]

On Fri, Jul 13, 2012 at 10:56 AM, Ryosuke Niwa <rniwa at webkit.org> wrote:

> On Fri, Jul 13, 2012 at 5:57 AM, Stephen Chenney <schenney at chromium.org>wrote:
>
>>  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 don't know how to review a patch and make sure all relevant comments are
> updated.
>
> As I have illustrated before, you can be modifying a function X, then a
> completely random function A which calls B that in turn calls C that in
> turns D ... that in turn calls X may have a comment dependent on the
> previous behavior of X without ever mentioning X. How am I supposed to know
> that there is such a comment?
>
> But the exact same thing can be said of code. Comments are not special in
this regard.

A simplistic example: You could be reviewing code that calls a method that
takes an int, but in practice that method should never take a negative
number. (it might even be documented correctly as such) And the only thing
you see in the diff is a call that might pass in a negative number due to
the way *it* is called. As a reviewer you do your due diligence but still
incorrectly assume that negative numbers are fine, perhaps because of the
way a function is named. (PositionAtRelativeZLevel or somesuch, which
seemed perfectly self documenting at the time, because N years ago
RelativeZLevels were never negative, by definition!)

Excess comments are far less likely to cause bugs than well-written code.
Or put another way, you can write excellent code and still have bugs just
as you can write excellent comments and still have typos or they get
outdated.

Yes, you do your due diligence as a reviewer, but there are limits
obviously - the code is written by humans who make mistakes. Reviews
drastically limit those mistakes, but so do comments.

I think the existence of clang demonstrates that syntax cant catch all pure
correctness issues - if anything the reviewer is a higher-level compiler,
who happens to also speak english. By rejecting comments, its as though you
have so much faith in the compiler and the
reviewer-as-language-interpreter, that one could be tempted to use it as a
crutch.

And yes while incorrect behavior can be observed through automated testing,
automated testing does not catch all incorrect behavior, especially
unexpected never-before-seen behavior. Why do you think people write
fuzzers? This is yet another way that folks are arguing "comments can be
wrong, code can't [thanks to compilers and automated testing], so don't
write excess comments"

Alec




> - Ryosuke
>
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo/webkit-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20120713/597b3c20/attachment.html>