95

I hate referencing paywalled content, but this video shows exactly what I'm talking about. Precisely 12 minutes in Robert Martin looks at this:

Some code

And says "One of my favorite things to do is getting rid of useless braces" as he turns it into this:

More code

A long time ago, in an education far far away, I was taught not to do this because it makes it easy to introduce a bug by adding another indented line thinking it's controlled by the if when it's not.

To be fair to Uncle Bob, he's refactoring a long Java method down to tiny little functions that, I agree, are far more readable. When he's done changing it, (22.18) it looks like this:

Yet more code

I'm wondering if that is supposed to validate removing the braces. I'm already familiar with the best practice. Can Uncle Bob be challenged on this point? Has Uncle Bob defended the idea?

  • 6
    I'm voting to close this question as off-topic because any answer to this is going to be "no" or "yes, and here's a citation." – Blrfl Jun 3 '16 at 23:46
  • 35
    Give it a few years, maybe he'll remove the useless linebreak too, and "if (see(something)) say(something);" will actually be a line of code ;-) – Steve Jessop Jun 4 '16 at 2:48
  • 8
    @SteveJessop Nice to hear that I'm years ahead. :D Without the newline, there's no risk of the infamous bug. Adding/removing braces as needed is an additional overhead, but much more gets saved when reading. – maaartinus Jun 4 '16 at 3:33
  • 9
    Uncle Bob makes some good points for doing it in Clean Code, page 35. If an if block is only ever one line, it doesn't matter. If you need to add more lines, it should be another function which reduces the if block back to one line (the function call). If you adhere to that, then braces simply do not matter -- at all. – user22815 Jun 4 '16 at 5:13
  • 13
    he should just do return (page==null) ? "" : includePage(mode,page); if he's that much into getting terse... I've thought no-brace style is cool until I started developing apps professionally. Diff noise, possible typo bugs etc. The braces, being there all the time, save you the time & overhead you'd need to introduce them later on. – vaxquis Jun 4 '16 at 14:15

10 Answers 10

47

Readability is no small thing.

I'm of a mixed mind when it comes to braces that enclose a single method. I personally remove them for things like single-line return statements, but leaving such braces out did in fact bite us very hard at the last place where I worked. Someone added a line of code to an if statement without also adding the necessary braces, and because it was C, it crashed the system without warning.

I never challenge anyone who is religious about always using braces after that little fiasco.

So I see the benefit of readability, but I am keenly aware of the problems that can arise when you leave those braces out.

I wouldn't bother trying to find a study or someone's published opinion. Everybody has one (an opinion, that is), and because it's a stylistic issue, one opinion is just about as good as any other. Think about the issue yourself, evaluate the pros and cons, and make up your own damned mind. If the shop you work for has a coding standard that covers this issue, just follow that.

  • 7
    I was bit by this too recently, when the indenting was off and misleading. – Andy Jun 4 '16 at 0:47
  • 12
    Because it was C without GCC 6's -Wmisleading-indentation. – wchargin Jun 4 '16 at 5:03
  • 2
    @CandiedOrange: It is Uncle Bob who is challenging the established dogma. – Robert Harvey Jun 4 '16 at 5:11
  • 1
    @RobertHarvey Didn't I make that clear? Yes, he's challenging dogma. He's challenging MY dogma. I'm just trying to be a good student and at least consider it. But Uncle Bob is so damn charismatic it makes me wonder if I'm losing objectivity. So I'm begging for help to find an antidote before I drink the koolaid. – candied_orange Jun 4 '16 at 5:19
  • 1
    @CandiedOrange: It absolutely is a context thing. The ones who just blindly accept the dogma do not consider context; they're the ones that still use the "one return only" rule in managed languages, long after the rule has outlived its usefulness and has actually become an impediment. – Robert Harvey Jun 4 '16 at 19:19
133

You can find several published promotions or rejections of no-brace styles at here or here or wherever bike sheds are painted.

Stepping away from the bike sheds, remember the great OS X/iOS SSL bug of 2014?

if ((err = SSLHashSHA1.update(&hashCtx, &serverRandom)) != 0)
    goto fail;
if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
    goto fail;
    goto fail;
if ((err = SSLHashSHA1.final(&hashCtx, &hashOut)) != 0)
    goto fail;

Yep, "caused" by no-brace blocks https://www.imperialviolet.org/2014/02/22/applebug.html

Preferences may depend on the brace style. If I had to write

if (suiteSetup)
{
    includePage(mode, suiteSetup);
}

I might be inclined to save space too. But

if (suiteSetup) {
    includePage(mode, suiteSetup);
}

only uses one "extra" line.


I know you didn't ask, but if I'm working alone, I'm a heretic. If I remove braces, I prefer

if (suiteSetup != null) includePage(mode, suiteSetup);

It doesn't have the same visual problem as the iOS SSL-style bug, but saves even more "unnecessary" lines than Uncle Bob does ;) Reads well if you're used to it, and has mainstream usage in Ruby (includePage(mode, suiteSetup) unless suiteSetup.nil?). Oh well, just know that there are a lot of opinions.

  • 13
    Plus, if you use } else[if(...)] {, that doesn't add any additional extra lines, so it adds up to only one extra line across the whole thing. – Random832 Jun 4 '16 at 5:59
  • 12
    I'm glad you bring up the iOS SSL bug. Ever since then, I've found myself using braces in such situations more and more. – Zach Lipton Jun 4 '16 at 9:21
  • 4
    Regarding the "goto fail" bug, it's worth noting that other aspects of Uncle Bob's coding style would have prevented it, too, most importantly his strong preference for extremely short methods. He would never have tolerated a 79-line method in the first place, and had the method been broken into smaller ones (a) the merge conflict that caused the duplication would probably never have happened and (b) the simpler flow control in the method would likely have meant that compiler warnings for unreachable code would have been generated that should have prevented the issue. – Jules Jun 4 '16 at 16:48
  • 16
    "goto fail" wasn't caused by single line blocks, it was caused by sloppy programming, even sloppier code review, and by not even once stepping through the code manually. – gnasher729 Jun 5 '16 at 7:35
  • 35
    Meh, lack of braces didn't cause that bug. Appalling programmers (and lack of any meaningful code review, testing) did. Fix the problem by firing those responsible, not by tying the hands of the rest of us! – Lightness Races in Orbit Jun 5 '16 at 13:29
61

Uncle Bob has many layers of defense against such a mistake that were not as commonplace when "always use braces" was the prevailing wisdom:

  1. A strong personal preference for single-line conditionals, so multi-line ones stand out and receive extra scrutiny.
  2. An editor that automatically outdents when you insert a second line.
  3. A complete suite of unit tests that he runs constantly.
  4. A complete suite of integration tests.
  5. A peer review done before his code is merged in.
  6. A rerun of all the tests by a continuous integration server.
  7. Testing done by a product quality department.

I think if someone did publish a challenge to Uncle Bob, he would have a pretty good rebuttal with the above points. This is one of the easiest mistakes to catch early.

  • 16
    I have never feared crashes the way I fear things that fail silently. At least you know when a crash happens. The back of my brain tingles when I see this stuff. It tingles the same way it does when I see while(true);{break;}. If there really is a valid context for this, it's going to take me a while to get over it. – candied_orange Jun 4 '16 at 5:05
  • 9
    Do we have an automated way of checking that includePage() is not a badly written macro with more than one statement? – Martin York Jun 4 '16 at 7:09
  • 51
    @LokiAstari Yes, it's called "Java does not have macros". – svick Jun 4 '16 at 11:35
  • 11
    All of the above are more "ways to avoid that the downfalls related to the 'useless braces' policy hurts me" than actually "reasons to use the 'useless braces policy". The facts that you need those (specially #1) should be considered more of a disavantage than an advantage. – SJuan76 Jun 4 '16 at 12:43
  • 16
    @Jules Oh yes! ALL the products I have received or released at work have 100% test coverage (at the very least), are peer reviewed (several times) and all the business around have Software QA departments. Yes. I guess you have found the same in your professional career, because every business has teams of programmers instead of single programmers, and they give them more than enough time to do all the testing they would like to do. Yes that describes all workplaces perfectly. Why should be worry about adding some additional bugs when we are sure our software ALWAYS ships 100% bug free? – SJuan76 Jun 4 '16 at 17:59
31

For the most-part this is personal preference, however there are some things to consider.

Possible Bugs

While it can be argued that bugs caused by forgetting to add-in braces are rare, from what I've seen that they do happen occasionally (not to forget the famous IOS goto fail bug). So I think this should be a factor when considering your code style (some tools warn about misleading-indentation, so it depends on your tool chain too).

Valid Code (that reads like it might be a bug)

Even assuming your project doesn't suffer from such bugs, when reading code you may see some blocks of code that look like they could be bugs - but aren't, taking some of your mental cycles.

We start with:

if (foo)
    bar();

A developer adds a useful comment.

if (foo)
    // At this point we know foo is valid.
    bar();

Later on a developer expands on it.

if (foo)
    // At this point we know foo is valid.
    // This never fails but is too slow even for debug, so keep disabled.
    // assert(is_valid(foo));
    bar();

Or adds a nested block:

if (foo)
    while (i--) {
        bar(i);
        baz(i);
    }

Or uses a macro:

if (foo)
    SOME_MACRO();

"... Since macros may define multiple lines of code, does the macro use do {...} while (0) for multiple lines? It should because its in our style-guide but I better check just in case!"


The examples above are all valid code, however the more content in the code-block, the more you need to read to ensure there aren't any mistakes.

Maybe your code-style defines that multi-line blocks require a brace (no matter what, even if they're not code), but I've seen these kinds of comments being added in production code. When you read it, there is some small doubt that whoever last edited those lines forgot to add a brace, sometimes I feel the need to double-check is working as intended (especially when investigating a bug in this area of the code).

Diff Noise

One practical reason to use braces for single lines is to reduce diff noise.

That is, changing:

if (foo)
    bar();

To:

if (foo) {
    bar();
    baz();
}

... causes the conditional line to show up in a diff as being changed, this adds some small but unnecessary overhead.

  • the lines show up as being changed in code-reviews, if your diffing tools are word-based you can easily see that only the brace changed, but that takes more time to check then if the line didn't change at all.
    Having said that, not all tools support word-based diffing, diff (svn, git, hg... etc) will show as if the entire line changed, even with fancy tools, sometimes you may need to quickly look over a plain line-based diff to see what changed.
  • annotation tools (such as git blame) will show the line as being changed, making tracking the origin of a line more step to find the real change.

These are both small, and depend on how much time you spend in code-review or tracking-down which commit changed lines of code.

A more tangible inconvenience of having extra lines changes in a diff, theirs higher likely-hood that changes in the code will cause conflicts which merging and need to be manually resolved.


There is an exception to this, for code-bases that have { on its own line - it's not a problem.

The diff noise argument doesn't hold if you write in this style:

if (foo)
{
    bar();
    baz();
}

However this isn't such a common convention, so mainly adding to the answer for completeness (not suggesting projects should use this style).

  • 8
    I like the reduces diff noise commented that is a define plus. – Martin York Jun 4 '16 at 7:23
  • 2
    If only everyone who is mad about JSON had any consideration for diff noise! I'm looking at you, no-last-comma rule. – Roman Starkov Jun 5 '16 at 8:32
  • 1
    Huh, I hadn't considered the diff-noise benefit of the {-on-its-own-line style. I really hate that style, and dislike working on projects where that style is required. Maybe with that in mind, I'll find it a little less frustrating having to code that way. Code density is important. If you can see all of a function on your monitor at once, you can more easily grok the whole thing. I like to leave blank lines between separate blocks of code inside a function for readability (to group related statements), and being forced to waste space in other places makes the intended breaks weaker. – Peter Cordes Jun 5 '16 at 15:30
  • 1
    @peter-cordes, also not a fan of {-on-its-own-line style, only included it for the answers completeness. As for trusting macros to use do{}while(0), I find the problem with this is you can trust it nearly all the time. The problem is accidents do happen, errors can slip by code review... and bugs can be introduced that wouldn't have been otherwise. – ideasman42 Jun 6 '16 at 1:20
  • 2
    If we are listing potential bugs, the ability to comment out individual lines is another good thing. I was tracking down a bug that was caused by somebody blindly commenting out the body of a one-line if statement. The following statement was then conditionally executed, rather than unconditionally executed. – Eldritch Cheese Jun 6 '16 at 12:48
16

Years ago, I was brought in to debug some C code. The bug was crazy hard to find, but eventually it boiled down to a statement like:

if (debug)
   foo (4);

And it turned out that the person who had written it had defined foo as a macro. A macro with two lines of code in it. And of course, only the first of those two lines was subject to the if. (So the second line was executed unconditionally.)

This may be absolutely unique to C and its preprocessor — which does substitutions before compilation — but I've never forgotten it. That kind of thing leaves a mark on you, and why not play it safe — especially if you use a variety of languages and toolchains and can't be sure such shenanigans aren't possible elsewhere?

Now I indent and use braces differently from everyone else, apparently. For a single line if, I would do:

if (debug) { foo (4); }

so it doesn't take any additional lines to include the braces.

  • 6
    In that case, whoever wrote that macro is the one at fault. – gnasher729 Jun 4 '16 at 18:37
  • 6
    Writing C preprocessor macros correctly may not be intuitive, but it can be done. And it should be done, as gnasher729 points out. And your example is the reason why any macro that contains more than one statement must include its body in a do { ... } while(0) loop. This will not only ensure that it will always be treated correctly by enclosing if() statements, but also that the semicolon at the end of the statement is swallowed correctly. Whoever does not know about such simple rules, simply should not write preprocessor macros. Defending at the calling site is the wrong place to defend. – cmaster Jun 5 '16 at 13:04
  • 18
    @cmaster: True, but how macros (or other language features) are written by others is beyond my control. How I use braces is under my control. I'm definitely not saying this is an iron-clad reason to include braces, but it is something that I can do to defend myself against other people's mistakes so I do it. – Wayne Jun 5 '16 at 13:31
  • @gnasher729, who cares if its someone else's fault? If you're doing code-reviews and following some reasonable code standards, it might as well be your teams fault. And if it reaches users, they'll blame the organization releasing buggy software. – ideasman42 Jun 6 '16 at 15:52
  • 1
    Whoever came up with macros is the one at fault. – Neme Sep 3 '17 at 20:14
14

"Uncle Bob" is allowed to have his opinion, you are allowed to have your opinion. No need to challenge him.

If you want an appeal to authority, take Chris Lattner. In Swift, if statements lost their parentheses, but always come with braces. No discussion, it's part of the language. So if "Uncle Bob" starts removing braces, the code stops compiling.

Going through someone else's code and "getting rid of useless braces" is a bad habit. Only causes extra work when the code needs to get reviewed, and when conflicts are unnecessarily created. Maybe "Uncle Bob" is such an incredibly good programmer that he doesn't need code reviews? I wasted one week of my life because one top programmer changed "if (p != NULL)" to "if (! p)" without a code review, hidden in the worst possible place.

This is mostly a harmless style debate. Braces have the advantage that you can insert another line of code without adding braces. Like a logging statement, or a comment (if followed by comment followed by statement is just awful). statement on the same line as if has the practical disadvantage that you have problems with many debuggers. But do whatever you prefer.

  • 1
    I guess it's about "challenging" because UB (sic!) presents his opinions as facts - at least it strikes me as so when I listen to him. – vaxquis Jun 5 '16 at 12:07
  • 3
    Saying "Do whatever you prefer" is actually to agree with Uncle Bob on this because that's what he argues "it does not matter". In many languages this is a non issue. I had previously thought in all the ones where it is an issue you should ALWAYS use braces and hadn't seen anyone challenging that. Now here's Uncle Bob challenging it and I'm wondering if he's getting away with it because he works in such highly refactored tiny methods or because he's full of it. I hadn't thought of it as harmless exactly because of the kinds of bugs @PaulDraper mentions in his answer. – candied_orange Jun 5 '16 at 13:11
  • Re always: the syntactic noise is distracting and the extra "blank" line for the close brace adds a visual separator in the paragraph where the lines should not be split apart, further hindering readability. This is especially important in concise tiny blocks that you mention. – JDługosz Jun 6 '16 at 14:42
9

My reasons for not removing braces are:

  1. reduce decision fatigue. If you always use braces, you never have to decide whether you are going to need braces or not.

  2. reduce development drag: even if you strive to eventually extract all multiple lines of logic to methods, having to convert a braceless if to a braced if to add logic is an annoying bit of development drag. So there's the effort of removing the braces, and the effort of adding them again when you need more code. Tiny, but annoying.

0

A young co-worker has said that the braces which we see as redundant and superfluous are in fact helpful to him. I don't recall exactly why, but if it allows him to write better code quicker, that alone is reason to keep them.

Fwiw, we agreed on a compromise that putting them on one line doesn't make such short preconditions unreadable/distracting to me. E.g.

if (!param) { throw invalid_argument (blahblah); }
if (n < 2) { return 0; }
// real code for function starts here...

I also point out that these introductory preconditions are often control-flow statements for the body (e.g. a return) so the fear of adding a second statement that's meant to be under the same condition and forgetting to write braces is moot. A second statement under the condition would be dead code anyway and not make sense.

I suspect that the fluency in reading issue is caused by the person's brain parser being wired to have the braces as part of the conditional statement. That is not an accurate representation of the grammar of C++, but could be a side-effect of learning certain other languages first, where that is the case.

  • 1
    Uncle Bob probably thinks he writes better code quicker without them. – djechlin Jun 6 '16 at 0:27
  • 2
    As do I, and others fluent in C++. – JDługosz Jun 6 '16 at 4:15
  • 1
    "if it allows him to write better code quicker, that alone is reason to keep them" ++ I have a Jr. who said the same, thus, we keep them. – RubberDuck Jun 6 '16 at 11:36
  • ...and that's thus reason alone to do it the way Uncle Bob wants. – djechlin Jun 6 '16 at 16:05
-1

Having watched that video just now, I noticed that eliminating braces makes it easier to see where code still needs to be refactored. If you need braces, your code can probably be a bit cleaner.

Having said that, when you're in a team, eliminating braces will probably lead to unexpected behavior some day. I say keep them, and you'll save yourself a lot of headaches when things do go wrong.

  • this doesn't seem to offer anything substantial over points made and explained in prior 10 answers – gnat Oct 5 '16 at 10:01
-3

I was taught not to do this because it makes it easy to introduce a bug by adding another indented line thinking it's controlled by the if when it's not.

If your code is well unit-tested, this kind of "bug" would explode in the face.

Cleaning the code by avoiding any useless and ugly brackets is a practice I follow.

  • 9
    Of course, the reverse is true as well: if your code is bug-free, you don't need any unit-tests. The reality is that we live in an imperfect world where there are sometimes mistakes in code and sometimes mistakes in tests. (In my experience, the latter are actually more common.) So while tests certainly reduce the risk of bugs, this is no excuse for finding other ways to raise the risk right back up again. – ruakh Jun 6 '16 at 4:59
  • 3
    To add to ruakh's comment: it is not possible to 100% unit test a code. – BЈовић Jun 6 '16 at 7:46
  • Please come see my code ;) While practicing TDD, it's always possible to show this kind of bug very quickly. – Mik378 Jun 6 '16 at 8:01
  • @Mik378 that's provably not true. The point is, if you used braces, you wouldn't even have to worry about it. – GoatInTheMachine Oct 8 '18 at 12:24

protected by gnat Oct 5 '16 at 10:00

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.