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

> On Thu, Jan 27, 2011 at 4:27 PM, Simon Fraser <simon.fra...@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
cmar...@apple.com




_______________________________________________
webkit-dev mailing list
webkit-dev@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev

Reply via email to