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

[Maciej Stachowiak]

I used to dislike everything about this rule; my original preference  
before we codified our style guidelines was to always put braces  
around the body of a control statement, even if it is only one line.  
Over time, I've gotten used to it and I find this:

if (condA)
     doA();

more pleasant to read than this:

if (condA) {
     doA();
}

Even for if/else statements where only one branch is single-line, I'm  
ok with the official style rule, although I can't say I actively like  
it. So these two examples look ok to me:

if (condA)
     doA();
else {
     doB1();
     doB2();
}

if (condA) {
     doA1();
     doA2();
} else
     doB();

However, when there is a lengthy chain of if ... else if ... else  
conditionals and a few of the blocks in the middle happens to be only  
a single line each, then I tend to find the code harder to write, read  
and modify. This pattern often comes up in the parseMappedAttribute()  
implementations of Element subclasses.

Regardless of the specific outcome, I would strongly prefer to have a  
consistent rule for this than to make it a matter of taste. One  
possible conservative modification to the rule is that if you have a  
multi-block if ... else if ... else chain, then if even one body has  
braces, all should. I think that would tend to reduce rather than  
increase visual noise, as all the "else" keywords would line up where  
right now they look ragged.

I would also not object to bigger changes in the rule if that were  
truly the community consensus, but personally I would set the barrier  
pretty high for a rule change that would implicate large chunks of  
existing code, and I think the benefit is much lower for if/else  
statements with only two clauses.

Regards,
Maciej


On Dec 2, 2009, at 9:00 PM, Peter Kasting wrote:

> This is a followup to my thread yesterday regarding consistent  
> enforcement of the style guide.  Like Yong Li, I find the current  
> rule about braces on conditional arms to be suboptimal.  The current  
> rule is that one-line arms must not have braces.  This leads to  
> strange constructions like:
>
> if (foo) {
>   a;
>   b;
>   c;
>   // etc., very long body
> } else
>   x;
>
> ...or perhaps:
>
> if (foo)
>   a;
> else if (bar) {
>   b;
>   c;
> } else if (baz)
>   d;
> else if (qux) {
>   e;
>   f;
> }
>
> 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.
>
> In most places this will not differ from the existing code, so it  
> will not "cause the whole codebase to become invalid"; but it  
> prevents cases where the inconsistency leads (IMO) to lower  
> readability/safety.  (As a bonus for Chromium developers, it's  
> compatible with the Google style guide too, although it goes further  
> than that guide in order to make the correct style explicit in all  
> cases.)
>
> PK
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev