[webkit-dev] coding style and comments

[Dirk Pranke]

I agree with all of the sentiments below, except for possibly one or two cases.

Namely, sometimes you write code where the "what" is not obvious. In
that situation, it may not be possible to change the code to make it
more obvious, because you are bound by contract. For example, suppose
you are implementing an interface with a method called sort(). The
fact that you choose a quicksort or a mergesort is a useful one-line
comment, and in my opinion is probably better than just wrapping a
method called quicksort() and hoping it gets inlined away.

A slight variant of this is that you may be learning or
reverse-engineering code where the "what" is not clear (perhaps
because it was poorly rewritten). In the longer-term, ideally you'd
clean up the code, but in the short term it might be useful to capture
the "what" in comments until you can safely refactor things and delete
the comments.

For example, evmar at chromium.org commented on IRC that he often checks
out files from the tree, adds local comments to the file, to help keep
track of what the code does, but then deletes them prior to checking
his patches back in. He's a pretty good guy, so I am guessing that his
comments would be useful to others as well (a net improvement to the
code and an incremental step towards the refactoring described above),
and it's a shame that that effort is wasted.

Two other specific examples, from the Python code I alluded to in my
first message:

In Tools/Scripts/webkitpy/common/system, there are the "fileset.py",
"directoryfileset.py", and "zipfileset.py" files. They are otherwise
fine but have no comments in them, probably because the contributor
felt that comments were not "webkit style". However, it took me a
non-trivial amount of time -- and a brief conversation with Mihai, who
reviewed the code -- to understand the structure and rationale of the
code (what is a fileset for, etc.). A couple of file-level or
class-level comments would've eliminated that concern and may save
others the same time in the future.

Secondly, in zipfileset.py, there is an open() method (line 49). What
that method does is entirely non-obvious: It extracts a named member
of a zip file and saves it into a file named according to the
"filename" parameter. I think this routine is an example of something
that would benefit from both either a "what" comment (or a renaming of
the method, but it's not obvious to me what it should be renamed to,
perhaps extract_as or extract_and_save_as) and a "why" comment
(because we need to take the "-actual" files from the layout test
archives and rename them to the "-expected" versions.

-- Dirk

[ Note that this is not meant to be a mark against that code, which is
generally pretty good IMO. I would have similar feedback on virtually
any C++ file I could open up, I suspect. This just happened to be the
file that prompted the train of thought. ]

On Thu, Jan 27, 2011 at 3:46 PM, Darin Adler <darin at apple.com> wrote:
> On the WebKit project we like “why” comments, but not “what” comments.
> Comments that say what the next block of code does usually are not a good
> idea. Instead by using appropriate names and granularity we want the code
> itself to say what the comment would have.
> We also frown on “textbook” style comments. Long block comments that read
> like a manifesto about what a code or class will do aren’t typical in
> WebKit.
> It’s critical that comments contain information that the code does not,
> rather than repeating information that is already present in the code. And
> that comments are brief enough and easy enough to read that they are likely
> to be noticed and updated as the code changes.
> I don’t think this has much to do with specific programming languages.
> A good discussion of comments could revolve around some specific code
> sequences and a discussion of whether a particular comment is suitable. I’m
> not sure we can get very far in the abstract.
>     -- Darin
>