[webkit-dev] coding style and comments

Adam Barth abarth at webkit.org
Sun Jan 30 10:27:41 PST 2011


On Sun, Jan 30, 2011 at 7:40 AM, David Kilzer <ddkilzer at webkit.org> wrote:
> 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)?

We're not really adding that many comments.  Mostly we're breaking
down monolithic objects like FrameLoader into more understandable
bundles of state.  For example, NavigationScheduler is a lot more
understandable now that it's separate from FrameLoader:

http://trac.webkit.org/browser/trunk/WebCore/loader/FrameLoader.cpp?rev=48958
http://trac.webkit.org/browser/trunk/Source/WebCore/loader/NavigationScheduler.cpp

In that case, after breaking NavigationScheduler out of FrameLoader,
the code got completely re-written to actually use C++ inheritance
instead of faking it with structs.  A similar thing happened (although
perhaps not as successfully) with HistoryController, which also used
to be part of FrameLoader.

More recently, we've been removing redundant state.  For example,

http://trac.webkit.org/changeset/76702/trunk/Source/WebCore/loader/FrameLoader.cpp

which means deleting a bunch of comments warning folks about the
dangers of not keeping various pieces of state synchronized properly.
I could be wrong, but I don't think we've added very many comments at
all.

Personally, I use FIXMEs a lot in new code.  For example, in the
XSSFilter I'm working on, there are lots of FIXMEs that are notes
about things I haven't gotten around to implementing yet (e.g., that
the code needs to handle POST but doesn't yet) or ideas for
optimizations that would be premature at this point (e.g., an extra
memcpy that could be eliminated).

Adam


More information about the webkit-dev mailing list