Re: [webkit-dev] Proposing style guide change regarding braces on conditional arms
Date: Wed, 2 Dec 2009 21:00:59 -0800 From: Peter Kasting <pkasting@google.com>
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.
For the people thinking about this, it would be nice if the final policy minimizes (and does not increase) the likelihood of having to modify several lines of surrounding code after touching one line of code. I don't think this consideration has been raised. This is similar to a point Darin Adler raised a few months back with regard to lining up comments:
Things manually lined up in source code generally create a small maintainability problem. You can’t rename things without re-lining things up, and if you make other types code changes without noticing the lined-up comments the code ends up looking messy an peculiar.
( https://lists.webkit.org/pipermail/webkit-dev/2009-September/009814.html ) --Chris
On Thu, Dec 3, 2009 at 6:24 PM, Chris Jerdonek <chris.jerdonek@gmail.com>wrote:
For the people thinking about this, it would be nice if the final policy minimizes (and does not increase) the likelihood of having to modify several lines of surrounding code after touching one line of code. I don't think this consideration has been raised.
Do you mean "in the steady state of modifying the code base" or "while files aren't compliant with whatever change might get made"? I think you mean the former, but if so, I'm not sure what policy would serve you best. Ideally you'd want "always use braces", but failing that, each proposal has a different set of cases where you do/don't have to change as you touch things. PK
On Thu, Dec 3, 2009 at 10:17 PM, Peter Kasting <pkasting@google.com> wrote:
On Thu, Dec 3, 2009 at 6:24 PM, Chris Jerdonek <chris.jerdonek@gmail.com> wrote:
For the people thinking about this, it would be nice if the final policy minimizes (and does not increase) the likelihood of having to modify several lines of surrounding code after touching one line of code. I don't think this consideration has been raised.
Do you mean "in the steady state of modifying the code base" or "while files aren't compliant with whatever change might get made"?
I meant the former (though my alternative below also reduces the latter).
I think you mean the former, but if so, I'm not sure what policy would serve you best. Ideally you'd want "always use braces", but failing that, each proposal has a different set of cases where you do/don't have to change as you touch things.
I don't feel strongly about this, but I will provide an example to illustrate what I mean. The illustrative case is several "else if" clauses with braces -- only one of which exceeds one line. If a code change removes the excess lines in that one clause, the two rules being considered say you have to remove the braces from all the clauses -- even though the clauses are already lined up. And this change can go back and forth indefinitely. Several people are already saying they value alignment within individual if-else control structures more than they value the number of braces. So removing the braces from all the clauses in the example above seems secondary. An alternative policy is as follows: (1) If-else control structures must have either braces around all clauses or braces around no clauses. (2) A clause with more than one line must be surrounded by braces. A consequence of this policy is that if-else statements may gradually converge to braces, but this change would take place only as needed. --Chris
On Fri, Dec 4, 2009 at 12:13 AM, Chris Jerdonek <chris.jerdonek@gmail.com>wrote:
An alternative policy is as follows:
(1) If-else control structures must have either braces around all clauses or braces around no clauses.
(2) A clause with more than one line must be surrounded by braces.
This is actually the precise policy used by the Google C++ Style Guide, which governs the Chromium repository. The problem with this is that it fails to ensure consistency, and the difference between consistently using braces everywhere and consistently not using them on one-line bodies is significant enough that this comes up as a bone of contention in code changes. On Fri, Dec 4, 2009 at 6:30 AM, Adam Treat <treat@kde.org> wrote:
It seems we keep changing the style guide as fashion changes. It is meant to ensure consistency and readability. This is a purely subjective change and I think unwarranted.
Obviously if I agreed I wouldn't have proposed this change, but it's worth detailing why I disagree. You mention that the style guide is meant to ensure readability. It seems clear, then, that you would support changes to improve readability if they did not decrease consistency; the main problems are that readability is significantly subjective (see some of the expressed opinions in this thread), and changing the rules currently decreases consistency (at least over the short to medium term). I think safety and maintenance cost are also worth considering. My opinion is that this change (as well as Maciej's proposed variant) does not merely affect readability but also improves safety very slightly. (Requiring braces at all times would improve safety more, and decrease maintenance cost, but at perhaps some cost to readability.) I agree with you that our codebase should be consistent. Personally, I would prefer that when we make style guide changes, we actually change the codebase to comply with them. Most of the work here could probably be automated, and I think if we did this it would address the main concern you have. The counterargument to this is that it can obscure blame (probably moreso for a change like the namespace indenting one than this change, but noticeably in both cases). On the other hand, gradual changes obscure blame (gradually, at the changed locations) too, and it's already the case that when digging up the history of a block of code you need to repeatedly skip past uninteresting changes. On Fri, Dec 4, 2009 at 9:39 AM, Adam Treat <treat@kde.org> wrote:
I don't think we should be changing the style guide for anything besides clarifications of currently unwritten rules. No matter how the fashion may change or how developers may change. Changing the rules throws consistency out the window which is, I believe, the greatest benefit of having a style guide.
Again, I think the consistency argument would be addressed if we actually made code consistent at rule changes. Changing code to make it more readable by the current developer set, when that developer set changes in makeup or opinion, seems like a good thing to me, as it improves productivity and code quality, regardless of whether the change is subjective in nature. As an example I give you the SQLite codebase, which has a consistent set of style rules that are so different from any other code I have ever encountered that I struggle to read and patch the code (and have been told the same by SQLite developers here at Google). You may argue that such a case is far outside any cognitive burden of the rule in question in this thread, but tiny amounts of cognitive friction, spread over a large codebase with many developers and a long lifetime, add up. PK
participants (2)
-
Chris Jerdonek
-
Peter Kasting