[webkit-dev] coding style and comments

[Chris Marrin]

On Jan 28, 2011, at 9:36 AM, Peter Kasting wrote:

> On Thu, Jan 27, 2011 at 4:27 PM, Simon Fraser <simon.fraser at apple.com> wrote:
> I think we have a distinct lack of comments that help novices to understand the code. I feel that we almost have a "privileged few" mentality in some code; if you can't figure it out by reading the code, then you shouldn't be messing with it.
> 
> Indeed.  This sets a high barrier to entry when trying to learn about any nontrivial subsystem.
> 
> Even when functions are kept brief and well-named, local simplicity does not eliminate global complexity; in fact it can mask it, making the system appear saner than it really is.  In this sense, I disagree that "self-documenting" code is possible on a large scale (even though I am certainly a fan of making the small-scale units concise, clear, etc.).
> 
> Back when we were writing the initial Chromium port, Brett Wilson and I had to rewrite the Image subsystem three separate times, each time because we realized we'd gotten the ownership semantics wrong and thought we now understood things better.  In this case, a simple function that merely allocates an object is deceptively self-documenting, because it's clear what it does in a local sense, but not in the larger picture of how it fits into the rest of the codebase.
> 
> No one is suggesting that silly comments like "for (int i = 0; i < 10; ++i)  // Count to 10." are a good thing, but I have never had any reviewer objections to adding more detailed comments about subtle bits.  At the very least I agree with Simon's suggestion of class-level comments, especially w.r.t. ownership.

I guess I fall somewhere in the middle, I've worked on a lot of projects where trying to use a 3rd party library is a waste of time because it is so poorly structured and documented that it would take more time understanding it and adapting it to your needs than just writing it from scratch. 

WebKit has a learning curve to be sure. I think it would be helpful to have some overview documents that describe the internals of different parts of the engine. But as far as inline comments go, I think the fewer the better. I've learned new programming conventions working on WebKit like using the most descriptive function name possible (even if it gets a bit wordy), using enums rather than booleans, and short circuit returns. These are all things that make the code much more readable. In a way, if you need a comment to explain something that's happening in a function you've failed! That's not always true of course. But hopefully the code in most cases is self explanatory and comments are just for the tricky bits.

The WebKit review process is also a great tool for keeping code clear. The reviewer is not going to have the context you have had in fixing the bug, and if it's not clear to him or her, they'll say it. When I review a patch for submission I always try to look at it through the eyes of a reviewer (yes I mean you, Simon :-)

Anyway, I think the coding conventions work pretty well as is. There's always room for improvement. But I for one have never worked on a codebase of this quality before. 

-----
~Chris
cmarrin at apple.com