[webkit-dev] Proposing style guide change regarding braces on conditional arms

[Peter Kasting]

On Wed, Dec 2, 2009 at 9:19 PM, Mark Rowe <mrowe at apple.com> wrote:

> On 2009-12-02, at 21:00, Peter Kasting wrote:
>
> > I find this tricky to read and error-prone.  I propose that the rule be
> modified to be:
> >
> > * When all arms of a conditional or loop are one physical line, do not
> use braces.  If any arms are more than one physical line (even if they are
> one logical line), use braces on all arms.
>
> I do not agree that this would be an improvement.
>

Are you satisfied with the existing rule, then?  If so, you would be the
first developer I have asked who is.  Admittedly my sample size is quite
small, but all those I've asked would prefer either a rule like my proposal
or a rule that says "braces always", which I find to be consistent and easy
to remember but somewhat verbose (and with a greater impact on the existing
code, for what that's worth).

> In most places this will not differ from the existing code, so it will not
> "cause the whole codebase to become invalid";
>
> I glanced briefly at three files: RenderObject.cpp, Element.cpp and
> Node.cpp.  In these three files I counted over two dozen places that would
> require modification to conform with this new rule.  That's by no means a
> majority of the relevant statements in these files, but it's not a small
> number either.


I stand by my statement.  In Element.cpp, 9 conditionals violate this out of
roughly 200 conditionals and loops.  One of these violates the existing
style guide too.  "Not a majority" is a significant understatement.
 Especially when compared to our namespace change that affects nearly every
header, or the proposed case indenting change that affects every switch
statement.  That said, I don't think "doesn't change a large percentage of
the code" is a significant argument for a rule, it mostly speaks to how
people wouldn't need to massively shift their thinking when writing code
because the rules have changed.

PK
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-dev/attachments/20091202/3cdc2b38/attachment.html>