[webkit-dev] Fwd: coding style and comments

[Aaron Boodman]

On Mon, Jan 31, 2011 at 5:23 PM, Ryosuke Niwa <rniwa at webkit.org> wrote:
> On Mon, Jan 31, 2011 at 4:54 PM, Aaron Boodman <aa at chromium.org> wrote:
>>
>> It seems like the one line patch to C just broke A. It had a
>> dependency on the behavior of C that was worth documenting. Now you
>> have changed C and the behavior of A is probably wrong (or at least
>> wasteful).
>
> Not necessarily. X' might be better a behavior and Y might no longer be
> needed because of that.

That is exactly my point. Either way the one line change leaves orphan
code. Having a comment just improves your chances of finding it
(assuming the comment references the depended-upon code).

>> If you had the comment, at least a grep of the source would have found the
>> dependency and alerted you that it was worth looking at this call site.
>
> I don't think so.  How do I know that modifying C would have changed the
> behavior of A?

To be specific, I'm talking about this situation:

void doA() {
  // We don't need to frobulate here because doC() already did that.
}

void doB() {
  doC();
}

void doC() {
  .. complicated stuff ...
}

Now if someone comes along and changes doC, they will probably grep
for the call sites to update them. That grep would find the comment in
doA() too, bringing attention to the tricky indirect relationship.
Without the comment, the relationship would be harder to find.

> This was a very simple example with only one indirection,
> namely, B.  But in the example I posted earlier (moveParagraph), a function
> calls hundreds of thousands of functions and it's virtually impossible even
> to enumerate all functions depended by the function. Yet, we must worry
> about the side-effects caused by the function in a call site.
> And we have tons of functions like this in editing because of the nature of
> what it does. So I insist on my point that keeping comments up-to-date is
> really hard if not impossible.

I'm not suggesting there should be a rule that all side-effects are
commented, that is clearly ridiculous. It is a matter of judgement to
determine when something warrants explanation. Typically it would be
when something is working in a way that is surprising or unexpected.

For example, moveParagraph() ends up calling lots of functions that
have lots of side-effects. Yet those shouldn't be surprising to
someone familiar with how editing works at a high level (note: this is
where class-level comments are super useful), so they shouldn't
require a comment.

- a