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