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

[Kenneth Russell]

On Wed, Dec 2, 2009 at 11:35 PM, Maciej Stachowiak <mjs at apple.com> wrote:
>
> 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.

In the WebKit WebGL implementation I've frequently encountered the
case where the else clause in a single if/else is a one-liner, and I
find it both ugly and error-prone to have to remove its braces. I'd
really like to be able to use braces on both arms.

Perhaps existing code could be grandfathered in but new or modified
code required to conform to the rule Peter proposes.

-Ken

> 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
>
> _______________________________________________
> webkit-dev mailing list
> webkit-dev at lists.webkit.org
> http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
>