104

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?

Thomas Owens
  • 79,623
  • 18
  • 192
  • 283
candied_orange
  • 102,279
  • 24
  • 197
  • 315
  • 2
    The underlying theme for styles such as this is that each line of code should read like an English sentence. – rwong Jun 04 '16 at 00:54
  • 37
    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 04 '16 at 02: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 04 '16 at 03:33
  • 2
    For that matter, one of my favorite things to do is to make it so that I can read code more quickly. So I'm in strong favor of eliminating the useless "` != null`" bit. The underlying assumption is that we're seeking truth of what is left. I've heard the argument about explicit text being easier to read. I prefer the idea that professionals know their language well. Since I tend to actually read such characters (in my head) as "is not null", eliminating the useless words actually helps me get through the code more quickly. However, styles often tend not to be right/wrong, but preference. – TOOGAM Jun 04 '16 at 03:46
  • 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_. –  Jun 04 '16 at 05:13
  • 1
    @TOOGAM I agree that comparison against null is excess verbiage, and in c++ is very much counter to best practices given smart pointers etc. But that code doesn't look like c++ for other reasons, and might be in a language that *doesn't* have implicit truth values for non-boolean types. – JDługosz Jun 04 '16 at 06:10
  • 3
    One case where Bob is surely wrong is if there is a guideline to always have braces (e.g. the KDE code base). In that case I believe consistency is better than slightly more readable code. (Yeah Bob is probably against guidelines too but in a project like KDE where you have hundreds+ developers, some of which are "amateurs" or non-professionals they give a consistent look at the code and actually help maintain it). – Bakuriu Jun 04 '16 at 08:56
  • Your question is "has anyone challenged this?" which should have a yes/no answer, but all of the posted answers are challenging it themselves...? – user253751 Jun 04 '16 at 09:38
  • @immibis Yes they are (well some, others defend Bob). I'm wishing that 1) they all focused on Bob's rarefied context so we don't just rehash old arguments. 2) They cited 3rd party arguments in the same vane so this doesn't turn into an opinion poll. – candied_orange Jun 04 '16 at 13:05
  • @Snowman With a decent quote that's worthy of being an answer. – candied_orange Jun 04 '16 at 13:10
  • @SteveJessop `if(see(something)) say(something)` with no break, has more impact if you reproduce it for us without letting word wrap add the break back. – candied_orange Jun 04 '16 at 13:24
  • 16
    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. –  Jun 04 '16 at 14:15
  • @Blrfl: My answer is a heart-warming anecdote, so I'd disagree. – Wayne Jun 04 '16 at 15:00
  • @CandiedOrange no, that is not an answer to the question of "Has anyone challenged Uncle Bob on his love of removing “useless braces”?" it is just a comment on _why_ he did that. Also, the question is off-topic and I cannot answer now anyway. –  Jun 04 '16 at 15:12
  • This question is [discussed on meta](http://meta.programmers.stackexchange.com/questions/8072/salvaging-has-anyone-challenged-uncle-bob-on-his-love-of-removing-useless-brac) – candied_orange Jun 04 '16 at 15:43
  • @Blrfl I'm looking for more than yes and no. I want a thoughtful cited answer on why the rule should be applied even if working like Uncle Bob does so we don't just rehash old arguments. – candied_orange Jun 04 '16 at 16:04
  • since it's on hold, i can't add an answer, but that Uncle Bob is wrong. i posted [an answer](http://programmers.stackexchange.com/questions/277067/how-are-basic-functions-implemented-in-a-programming-language-if-they-are-not-bu/277071#277071) that shows the [simplest, most consistent, and most readable indentation scheme and it "uses braces"](https://en.wikipedia.org/wiki/Indent_style#Whitesmiths_style). – robert bristow-johnson Jun 04 '16 at 18:01
  • If Uncle Bob were to use Rust, he would be disappointed, since there braces are *not optional* even for a single statement (the parens around the condition is, though). – dureuill Jun 05 '16 at 08:58
  • @Snowman I just carefully looked over page 35 of Clean Code (first printing) and I find an example of going without braces after an `if` but I couldn't find any "points" arguing for doing this. He never even uses the word "braces" on this page. He's talking about making functions small and doing one thing. I can't find these points anywhere in the book. We looking at the same thing? – candied_orange Jun 05 '16 at 12:54
  • @dureuill if we're going to take other languages into account, then I'd say _if Uncle Bob were to use Python or Haskell_ he'd be delighted, since these languages never need braces thanks to indentation-guided syntax. – leftaroundabout Jun 05 '16 at 15:10
  • 1
    @CandiedOrange the point was implicit, where he compares two code examples differing by "compound block with multiple statements" and "compound block with a single statement, no braces, calling a function." –  Jun 05 '16 at 15:13
  • @Snowman I'm looking at Clean Code, page 35. I see the headings "Blocks and Indenting" and "Do one thing". I see listing 3-3. I don't see any of your quotes about "compound blocks". Again, are we looking at the same thing? – candied_orange Jun 05 '16 at 15:28
  • @CandiedOrange "How short should your functions be? They should usually be shorter than Listing 3-2! Indeed, Listing 3-2 should really be shortened to Listing 3-3." Now do a mental `diff` between 3-2 and 3-3 and report back. –  Jun 05 '16 at 15:30
  • @Snowman That quote I do see. I also see the example I already mentioned in listing 3-3 that shows Bob going without braces. I still don't see the points you promised would argue that it's ok to go braceless. I see arguments to keep functions small. I guess I completely misunderstood you when you said "Uncle Bob makes some good points for doing it". I just see him hauling off and doing it. – candied_orange Jun 05 '16 at 16:07
  • 2
    this whole debate is rather peculiar from the perspective of someone working with languages like haskell where statements don't even exist, so you couldn't possibly put 2 of them after eachother, expecting both to go in the same "code block". – sara Jun 05 '16 at 16:12
  • 1
    I'm not sure if it's the same context but Douglas Crockford (of JSON and AJAX fame) strongly recommends having braces instead of removing them. To the extreme of JSLint throwing an error (warning?) if you leave out braces because according to him it's bad style that invites future bug. And it's not configurable in JSLint - you MUST put in the braces if you want your code to pass linting (other linters make this a configurable thing). – slebetman Jun 05 '16 at 17:27
  • @slebetman the context I meant, but have apparently failed to communicate, is in the small (and in java). Small as in 4 lines is considered a big function. Uncle Bob writes code like I've rarely seen in production. I'm objecting to his removing braces even in his context but acknowledging that I have read very few things in that context that speak to this "useless braces" issue. – candied_orange Jun 05 '16 at 17:35
  • @TOOGAM This question is tagged as Java, and Java requires conditionals to be of type boolean, so removing `!= null` would cause a compilation error. – user2752467 Jun 06 '16 at 06:36
  • I would challenge Bob to go further. If you don't like braces. Use [Ternary Operator](https://en.wikipedia.org/wiki/%3F:). – Laiv Jun 06 '16 at 06:49
  • 1
    @Laiv I agree, I very much prefer the ternary operator. It discourages side effects, and it's just an expression, so both branches must return a value of the same type. It's much closer to a "functional if". It also promotes somposition and never putting more than a single row in each branch. People say ternaries are bad because with large ifs they get hard to read. To that I say, well then don't make large ifs! – sara Jun 06 '16 at 07:20
  • 2
    Curly braces are not syntactic sugar since more than one line without curly braces acts differently than more than one line with curly braces. I'm with [Douglas Crockford](http://jslint.com/help.html) on this one. – Michael Shopsin Jun 06 '16 at 14:47
  • I remove single-line block braces, unless I'm using javascript, because javascript be crazy. – Mark Rogers Jun 06 '16 at 14:49

10 Answers10

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.

Paul Draper
  • 5,972
  • 3
  • 22
  • 37
  • Ye Gods that PS takes me back. I did exactly that banging away at home before I knew what a style guide was. I wonder how likely it really is that someone can ignore the call on the same line and put indented code under it anyway. – candied_orange Jun 04 '16 at 05:13
  • @CandiedOrange, it's a tinsey bit like Ruby `includePage(mode, suiteSetup) unless suiteSetup.nil?` – Paul Draper Jun 04 '16 at 05:23
  • 14
    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 04 '16 at 05:59
  • Regarding the code in your PS section: `#define includePage(...) incPage_forReal(__VA_ARGS__); sideEffect()` Sure that macro is bad, but such things happen. – Daniel Jour Jun 04 '16 at 07:48
  • 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 04 '16 at 09:21
  • 3
    Remember the alternate-universe iOS SSL bug where they had braces around all their `if` bodies, which shifted things enough that the duplicated line of code was a different one, which happened to *still* break certificate validation? – user253751 Jun 04 '16 at 09:36
  • 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 04 '16 at 16:48
  • 2
    Additionally, as a strong proponent for automated testing, he would undoubtedly have tests for all the standard failure conditions that run automatically on checkin, which even had the problem not been spotted earlier would have led to a failure at that point. Yes, always using braces could have avoided "goto fail", but there are plenty of other practices that could have achieved the same thing too. – Jules Jun 04 '16 at 16:51
  • 1
    @Jules, yes, the salient concept is "defense in depth". Errors can be caught via readability patterns, code review, static checks, runtime tests, etc. Technologies, organizations, and programmers make various choices of which to focus on. – Paul Draper Jun 04 '16 at 17:45
  • i am unfamiliar with the idiom of "bike sheds" or "wherever bike sheds are painted". i guess i can glean it from context, but i would rather ask; *what does it mean?* – robert bristow-johnson Jun 04 '16 at 18:07
  • @robertbristow-johnson: https://en.wikipedia.org/wiki/Law_of_triviality, http://bikeshed.com/, http://personalexcellence.co/blog/bike-shed-effect/ – Paul Draper Jun 04 '16 at 18:10
  • 1
    thanks, Paul. it is as i suspected and that's fine. personally, when several people work on the same project, i am tolerant of other indentation schemes that are consistent where other people can decently read your code with whatever editor they like to use. there was a guy who insisted that emacs was god and tabs set to 8 spaces and he liked indenting 2 spaces per indent level (but emacs puts in a tab at the level-4 indent). can you imagine the trouble i had reading his code? that was not "**just**" a pointless bike shed color issue. – robert bristow-johnson Jun 04 '16 at 18:27
  • 3
    @PaulDraper: The one-line style is in use at the place I currently work. I must admit, it took awhile to get used to. It is the one place that I insist on a line-length limit; if you can't manage to get your `if` and your enclosing statement into 120 characters or so, you still need to put the enclosing statement on a separate line. – Robert Harvey Jun 04 '16 at 19:34
  • 2
    @RobertHarvey, couldn't agree more. A reasonable line limit must be obeyed. – Paul Draper Jun 05 '16 at 04:22
  • 18
    "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 05 '16 at 07:35
  • In this particular case I would be inclined to do `return (page != null) ? includePage(...) : "";` – CompuChip Jun 05 '16 at 09:10
  • 37
    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 05 '16 at 13:29
  • 3
    If they'd have had any sense, they would never have used goto to begin with and the whole bug would not have existed. – Mr Lister Jun 05 '16 at 14:02
  • (Note to self: never let Uncle Bob see my code. Not that I use goto, mind...) – Mr Lister Jun 05 '16 at 14:03
  • 1
    +1 for the one-line if formatting. – John R. Strohm Jun 05 '16 at 15:47
  • 1
    Similar to other people here, I am convinced this bug was not caused by lack of braces. Even without a code review by a second developer, the duplicate "goto fail" line is such an eye catcher - if the author missed to correct that line at first hand, he surely would have added a different bug even when he had used braces. Moreover, what crappy compiler did he use - any decent compiler should have give the author a warning about the unreachable code. – Doc Brown Jun 05 '16 at 19:49
  • Funny, I go for `if (suiteSetup != null) { includePage(mode, suiteSetup); }` all one line :-) – Ben Jun 06 '16 at 07:38
  • Due to my personal opinion "egyptian brackets" are evil. I like my opening and closing brackets aligned at the same tab position, to see where the block starts and ends. You can - of course - say "a block starts with an if and ends with a bracket", but it simply isn't true. A Block can start with "if", "while", "for", and I could go on and on. – mg30rg Jun 06 '16 at 12:33
  • @mg30rg: Most IDEs provide at least matching bracket indications, if not block indicators. I find the elimination of extra "essentially empty" lines to be worth it. If I'm working in a language that has dedicated `BEGIN` and `END` keywords, I do put those on separate lines, but for a single character it just seems wasteful. – TMN Jun 06 '16 at 17:08
  • **The comment section is not meant for extended discussions. Please take your discussions to our chat room if you would like to discuss the answer further. Thank you.** – maple_shaft Jun 07 '16 at 01:31
  • @TMN I have worked as a Delphi developer (it has dedicated `begin` and `end`), and of course wrote those in different lines too, but I don't think aligning the opening and closing of a code block at the same tab position. Actually the code block doesn't begin with "if", "while", "for", etc. but with a `{`. The enlisted keywords are symbol evaluators determining if the next _instruction_ will be performed, repeated, etc. The code block opener is there to note the programmer and compiler that instead of a single statement a code block will be executed upon that decision. – mg30rg Jun 07 '16 at 14:30
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.

candied_orange
  • 102,279
  • 24
  • 197
  • 315
Karl Bielefeldt
  • 146,727
  • 38
  • 279
  • 479
  • 2
    Optionally he might also have a linter that detects spurious indents, so that even if the poor sap who introduces the error doesn't use the same text editor as Bob, at test time or checkin time they still in effect get the benefit of the automatic outdent you mention. – Steve Jessop Jun 04 '16 at 02:43
  • 17
    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 04 '16 at 05:05
  • 1
    1: Not sure that is better. 2: Has everybody standardized on the same editor. 3: Yes thats's good 4: Yes that's good. 5: If your peer review is concentrating on this level of syntax they are doing it wrong (automated tools look for the silly spacing/bracing etc) reviewers should be checking higher level constructs. 6: Yes that's good. 7: Yes that's good. All the testing is good but it assumes the testing is perfect (is it ever). Also these rules are to prevent the less experienced of us from messing up when we have not managed to check all the boxes in the above list. – Martin York Jun 04 '16 at 07:07
  • 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 04 '16 at 07:09
  • 53
    @LokiAstari Yes, it's called "Java does not have macros". – svick Jun 04 '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 04 '16 at 12:43
  • 2
    Indeed; the auto-indenting editor completely knocks this challenge down by itself. – Joshua Jun 04 '16 at 13:47
  • 3
    @SJuan76 - if it were the case that you only do those things *because* you avoid useless braces, I'd agree, they're substantially worse than simply including the braces. But given that they're all things that we mostly do *anyway*, relying on the extra protection they provide to allow us to have tidier code isn't a huge issue is it? – Jules Jun 04 '16 at 16:55
  • 18
    @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 04 '16 at 17:59
  • 1
    @Jules and that does not account for #1, either, forcing me to multiply my methods (or overcomplicate my statements) without good reason. – SJuan76 Jun 04 '16 at 18:07
  • If only all of these things were practiced regularly. They are not. I still find developers at the Fortune 500 I work for that think they'll be "hardcore" and code directly against production on the server with barebones vim and no SCM backups. Ugg – Mike McMahon Jun 04 '16 at 22:44
  • At my place, we skip those braces and protect ourselves from mistakes by using "reformat entire file" on file save in Visual Studio. This unindents any stray statements. – Roman Starkov Jun 05 '16 at 08:30
  • @svick neither does C++. What C++ does is called "complex text replacement routines" and it's a horrible idea. For real macros, look at Lisp's ability to run arbitrary code at compile time. – John Dvorak Jun 05 '16 at 08:42
  • @SJuan76 well I think you'll be hard pressed to find any record of UB saying "remove all braces and never test your code", so I dunno what that has anything to do with it. of course, almost no codebase has automatic tests for every line of code, but if you ever change some code, don't even run it to test it manually, just push it to master and deploy, then i don't think the problem is the braces, but rather a crappy workflow. NEVER ship untested code, that's software engineering 101. – sara Jun 05 '16 at 16:05
  • 2
    Everyone's focusing on the downside--what about the upside that you now have more code on the screen? So long as you have an auto-indenting editor I'm with Uncle Bob. As far as I'm concerned it's a holdover from the days we didn't have such things and got results such as the iOS SSL bug. – Loren Pechtel Jun 06 '16 at 02:38
  • 13
    "_This is one of the easiest mistakes to catch early._" -- It's also one of the easiest mistakes to avoid entirely by typing two more characters :) – CompuChip Jun 06 '16 at 07:15
  • 1
    This answer is closer to what I had in mind when I asked the question. It shows the environment Uncle Bob works in. I was looking for challenges to Uncle Bob that took this environment into account (rather than just generically rehash the old braces issue). Not demanding particular conclusions. Just wanted analyses that explored both sides in this context. – candied_orange Jun 06 '16 at 13:53
  • 1
    Another defence against possible bugs related to braceless if conditions would be running a code style checker that will fail on wrong indent somewhere in the pipeline between commit and deployment. – bdsl Mar 25 '18 at 16:17
54

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.

Robert Harvey
  • 198,589
  • 55
  • 464
  • 673
  • 7
    I was bit by this too recently, when the indenting was off and misleading. – Andy Jun 04 '16 at 00:47
  • I like the braces around a one line `if` because it turns it into a mini-paragraph (instead of leaving it as a sentence) and, in my opinion, adds to the readability. What I do do, however, is put the starting brace at the end of the line with the `if` instead of on a line on its own. So the overall effect is a reduction of one line, with an (almost) empty line afterwards to separate it from the following code. – CJ Dennis Jun 04 '16 at 03:36
  • Robert Harvey I'm challenging myself on this issue. I have been that religious fanatic. It's precisely because people tend to not challenge this dogma that I'm second guessing myself. Ideally I like to see someone challenging Uncle Bob on this and see him respond. I have seen dogma fall before. `goto` used to be cool. Only one return from a method used to be cool. And in some contexts they still are (assembly still has jmp which is a form of goto. C still doesn't have exceptions so it's nice to have one return so there is a place to clean up resources). So, could this also be a context thing? – candied_orange Jun 04 '16 at 04:55
  • 12
    Because it was C *without [GCC 6's `-Wmisleading-indentation`](http://developers.redhat.com/blog/2016/02/26/gcc-6-wmisleading-indentation-vs-goto-fail/)*. – wchargin Jun 04 '16 at 05:03
  • 2
    @CandiedOrange: It is Uncle Bob who is challenging the established dogma. – Robert Harvey Jun 04 '16 at 05: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 04 '16 at 05: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 04 '16 at 19:19
  • I find that this commit is the most astounding example I've ever found in favor of curly braces in a single-statement if: https://github.com/mono/xwt/commit/e1275a299522ff4db93e4b464a81211d192816c1?diff=split – Hay Oct 10 '16 at 19:49
  • That's why you use an IDE. – Neme Sep 03 '17 at 20:07
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).

ideasman42
  • 873
  • 6
  • 18
  • I honestly hadn't ever considered this. It's a reasonable point. I once worked in a shop where the diff tool wasn't smart enough to see all whitespace as the same. When a programmer changed a few scattered lines and ran a reformat to ensure all tabs were removed it drove the reviewer completely up the wall. – candied_orange Jun 04 '16 at 06:16
  • Yes, it can be quite annoying. Even with tools that ignore white-space, adding/removing a brace isn't a white-space change, so it will show up irrespective. – ideasman42 Jun 04 '16 at 06:38
  • 8
    I like the reduces diff noise commented that is a define plus. – Martin York Jun 04 '16 at 07: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 05 '16 at 08: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 05 '16 at 15:30
  • Anyway, I think a lot fewer people will claim it's perfectly fine to omit the braces when an `if` is controlling a `for` or `while`, or even a multi-line comment, vs. the trivial case of controlling a simple statement. The macro case depends on the code-base. If you can't trust that the macros properly use `do{}while(0)` when needed, just use braces. (As long as it's not the space-wasting kind!) – Peter Cordes Jun 05 '16 at 15:34
  • 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 06 '16 at 01: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 06 '16 at 12:48
  • "Diff noise" I consider that a reason to put braces on new lines. – Neme Sep 03 '17 at 20:12
15

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.

Wayne
  • 251
  • 1
  • 6
  • 6
    In that case, whoever wrote that macro is the one at fault. – gnasher729 Jun 04 '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 - reinstate monica Jun 05 '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 05 '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 06 '16 at 15:52
  • 1
    Whoever came up with macros is the one at fault. – Neme Sep 03 '17 at 20:14
  • @gnasher729, you can talk about "fault" all day long, but that doesn't make your car any less mangled from the train collision. – Paul Draper Oct 08 '18 at 03:01
15

"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.

gnasher729
  • 42,090
  • 4
  • 59
  • 119
  • 3
    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. –  Jun 05 '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 05 '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 06 '16 at 14:42
10

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.

Michael Johnston
  • 201
  • 1
  • 3
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.

JDługosz
  • 568
  • 2
  • 9
-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.

Gijs Overvliet
  • 99
  • 1
  • this doesn't seem to offer anything substantial over points made and explained in prior 10 answers – gnat Oct 05 '16 at 10:01
-2

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.

Mik378
  • 3,838
  • 7
  • 33
  • 60
  • 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 06 '16 at 04:59
  • 3
    To add to ruakh's comment: it is not possible to 100% unit test a code. – BЈовић Jun 06 '16 at 07:46
  • Please come see my code ;) While practicing TDD, it's always possible to show this kind of bug very quickly. – Mik378 Jun 06 '16 at 08: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 08 '18 at 12:24