[webkit-dev] coding style and comments
abarth at webkit.org
Sun Jan 30 10:56:43 PST 2011
As an example of a harmful comment, this comment just wasted 15
minutes of my time:
The comment was true when written (although confusing because I
thought it meant that these functions are virtual so we should avoid
calling them and using encoding() directory), but the comment is no
longer meaningful because Document::encoding is no longer virtual.
IMHO, the maintenance burden of some comments often out-weighs their
value. Some comments can be valuable. For example, this comment has
been helpful to me in the past:
It saved me a bunch of time digging through the history of FrameLoader
to understand why that code was written the way it was.
On Sun, Jan 30, 2011 at 10:27 AM, Adam Barth <abarth at webkit.org> wrote:
> 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:
> 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,
> 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
> 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).
More information about the webkit-dev