[webkit-dev] coding style and comments

[David Kilzer]

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.

An interesting case study would be the (still ongoing?) refactoring of the loader code over the past few months (thanks to Adam Barth and others).   Is it easier to understand now than before the refactoring started?  How many comments were added in the process (FIXMEs notwithstanding)?

Dave

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20110130/c6dae4c2/attachment.html>