Unnecessary curly braces in C++
Asked Answered
A

14

213

When doing a code review for a colleague today I saw a peculiar thing. He had surrounded his new code with curly braces like this:

Constructor::Constructor()
{
   // Existing code

   {
      // New code: do some new fancy stuff here
   }

   // Existing code
}

What is the outcome, if any, from this? What could be the reason for doing this? Where does this habit come from?

The environment is embedded devices. There is a lot of legacy C code wrapped in C++ clothing. There are a lot of C turned C++ developers.

There are no critical sections in this part of the code. I have only seen it in this part of the code. There are no major memory allocations done, just some flags that are set, and some bit twiddling.

The code that is surrounded by curly braces is something like:

{
   bool isInit;
   (void)isStillInInitMode(&isInit);
   if (isInit) {
     return isInit;
   }
}

(Don't mind the code, just stick to the curly braces... ;) ) After the curly braces there are some more bit twiddling, state checking, and basic signaling.

I talked to the guy and his motivation was to limit the scope of variables, naming clashes, and some other that I couldn't really pick up.

From my point of view this seems rather strange and I don't think that the curly braces should be in our code. I saw some good examples in all the answers on why one could surround code with curly braces, but shouldn't you separate the code into methods instead?

fsdf
Absorptance answered 14/3, 2012 at 14:39 Comment(13)
What was your colleague's answer when you asked him why he did it?Brownlee
Is this only in constructors or is it in other places in the code?Storm
Quite common with the RAII pattern. Quick overview: c2.com/cgi/wiki?ResourceAcquisitionIsInitializationClapper
Maybe a duplicate? https://mcmap.net/q/128573/-scope-with-brackets-in-cSelfannihilation
Maybe a duplicate? How about this from 2008? #249509Clapper
I hate unnecessary curly bracesPaisa
Were there any declarations in the inner block?Mohn
maybe he just wanted to easily 'fold' away that new section in his editorAlmsgiver
Return value from constructor, something newMong
Re. whether to construct a separate function for the code inside brackets: maybe you want the benefits of brackets (scoping) without the overhead of calling a function.Grenadine
@Mysticial, Marcin: also, this: stackoverflow.com/questions/3189366Heisenberg
I'd vote no to adding a function simply to group a small set of used-once minor operations. Funcs take a tiny bit longer to execute (build stack frame, etc.), so in inner loops that could cost you. Extra curly braces are fine, for whatever reason the author wanted them for.Disability
@PeterMortensen what is that fsdf in your edit? is it a typo or something that I don't understand yet?Perversity
A
317

It's sometimes nice since it gives you a new scope, where you can more "cleanly" declare new (automatic) variables.

In C++ this is maybe not so important since you can introduce new variables anywhere, but perhaps the habit is from C, where you could not do this until C99. :)

Since C++ has destructors, it can also be handy to have resources (files, mutexes, or whatever) automatically released as the scope exits, which can make things cleaner. This means you can hold on to some shared resource for a shorter duration than you would if you grabbed it at the start of the method.

Adi answered 14/3, 2012 at 14:41 Comment(9)
+1 for the explicit mentioning of new variables and old habitWarman
+1 for using block scope used to free resources as fast as possibleEncratia
It's also easy to 'if (0)' a block.Emboss
My code reviewers often tell me that I write too short functions/methods. To keep them happy and to keep myself happy (i.e. to separate out unrelated concerns, variable locality etc.), I use just this technique as elaborated by @unwind.Irascible
@ossandcad, they tell you your methods are "too short"? That's extremely hard to do. 90% of developers (myself likely included) have the opposite problem.Floe
C++ beginner here.. I don't understand what you mean by "new (automatic) variables"? How is it different from saying just "new variables"?Doorstop
@Doorstop It's different because you can have "new variables" that are not automatic, so you would need to manually allocate memory for them. (Using the "new" keyword, for example)Shaylynn
@Brhaka: I had to jog my memory to understand the response to years old comment. But thank you very much. Side Note: you can say if (!knowHowToSolve). :DDoorstop
@Doorstop Hahahha! Wow! That was a fast response. Yeah, I'm a little bit late, but I guess this can be useful for new visitors. Also, thanks for the suggestion! Back when I wrote the description I would prefer the explicit way, but nowadays I use the negation most of the times.Shaylynn
C
180

One possible purpose is to control variable scope. And since variables with automatic storage are destroyed when they go out of scope, this can also enable a destructor to be called earlier than it otherwise would.

Computation answered 14/3, 2012 at 14:40 Comment(4)
Of course, really that block should just be made into a separate function.Demetra
Historical note: This is a technique from the early C language which allowed creation of local temporary variables.Elswick
I have to say -- although I'm satisfied with my answer, it's really not the best answer here; the better answers mention RAII explicitly, since it's the main reason why you'd want a destructor to be called at a specific point. This seems like a case of "fastest gun in the West": I posted fast enough that I got enough early upvotes that I gained "momentum" to gain upvotes faster than some better answers. Not that I'm complaining! :-)Computation
@BlueRaja-DannyPflughoeft You're oversimplifying. "Put it in a separate function" isn't the solution to every code problem. The code in one of these blocks might be tightly coupled with the surrounding code, touching several of its variables. Using C functions, that requires pointer operations. Furthermore, not every code snippet is (or should be) reusable, and sometimes the code might make not even make sense on its own. I sometimes put blocks around my for statements to create a short-lived int i; in C89. Surely you're not suggesting that every for should be in a separate function?Vaso
G
104

The extra braces are used to define the scope of the variable declared inside the braces. It is done so that the destructor will be called when the variable goes out of scope. In the destructor, you may release a mutex (or any other resource) so that other could acquire it.

In my production code, I've written something like this:

void f()
{
   // Some code - MULTIPLE threads can execute this code at the same time

   {
       scoped_lock lock(mutex); // Critical section starts here

       // Critical section code
       // EXACTLY ONE thread can execute this code at a time

   } // The mutex is automatically released here

  // Other code  - MULTIPLE threads can execute this code at the same time
}

As you can see, in this way, you can use scoped_lock in a function and at the same time, can define its scope by using extra braces. This makes sure that even though the code outside the extra braces can be executed by multiple threads simultaneously, the code inside the braces will be executed by exactly one thread at a time.

Galliard answered 14/3, 2012 at 14:42 Comment(3)
I think it's cleaner just to have: scoped_lock lock(mutex) //critical section code then lock.unlock().Sharecrop
@szielenski: What if the code from the critical section throws exception? Either the mutex will be locked forever, or the code will not be that cleaner as you said.Galliard
@Nawaz: @szielenski's approach won't leave the mutex locked in case of exceptions. He also uses a scoped_lock that will be destructed on exceptions. I usually prefer to introduce a new scope for the lock as well, but in some cases the unlock is very useful. E.g. to declare a new local variable within the critical section and then use it later. (I know I'm late, but just for completeness...)Traitor
S
58

As others have pointed out, a new block introduces a new scope, enabling one to write a bit of code with its own variables that don't trash the namespace of the surrounding code, and doesn't use resources any longer than necessary.

However, there's another fine reason for doing this.

It is simply to isolate a block of code that achieves a particular (sub)purpose. It is rare that a single statement achieves a computational effect I want; usually it takes several. Placing those in a block (with a comment) allows me tell the reader (often myself at a later date):

  • This chunk has a coherent conceptual purpose
  • Here's all the code needed
  • And here's a comment about the chunk.

e.g.

{  // update the moving average
   i= (i+1) mod ARRAYSIZE;
   sum = sum - A[i];
   A[i] = new_value;
   sum = sum + new_value;
   average = sum / ARRAYSIZE ;  
}

You might argue I should write a function to do all that. If I only do it once, writing a function just adds additional syntax and parameters; there seems little point. Just think of this as a parameterless, anonymous function.

If you are lucky, your editor will have a fold/unfold function that will even let you hide the block.

I do this all the time. It is great pleasure to know the bounds of the code I need to inspect, and even better to know that if that chunk isn't the one I want, I don't have to look at any of the lines.

Sandarac answered 14/3, 2012 at 15:56 Comment(0)
W
24

One reason could be that the lifetime of any variables declared inside the new curly braces block is restricted to this block. Another reason that comes to mind is to be able to use code folding in the favourite editor.

Warman answered 14/3, 2012 at 14:41 Comment(0)
T
19

This is the same as an if (or while, etc.) block, just without if. In other words, you introduce a scope without introducing a control structure.

This "explicit scoping" is typically useful in following cases:

  1. To avoid name clashes.
  2. To scope using.
  3. To control when the destructors are called.

Example 1:

{
    auto my_variable = ... ;
    // ...
}

// ...

{
    auto my_variable = ... ;
    // ...
}

If my_variable happens to be a particularly good name for two different variables that are used in isolation from each other, then explicit scoping allows you to avoid inventing a new name just to avoid the name clash.

This also allows you to avoid using my_variable out of its intended scope by accident.

Example 2:

namespace N1 { class A { }; }
namespace N2 { class A { }; }

void foo() {

    {
        using namespace N1;
        A a; // N1::A.
        // ...
    }

    {
        using namespace N2;
        A a; // N2::A.
        // ...
    }

}

Practical situations when this is useful are rare and may indicate the code is ripe for refactoring, but the mechanism is there should you ever genuinely need it.

Example 3:

{
    MyRaiiClass guard1 = ...;

    // ...

    {
        MyRaiiClass guard2 = ...;
        // ...
    } // ~MyRaiiClass for guard2 called.

    // ...

} // ~MyRaiiClass for guard1 called.

This can be important for RAII in cases when the need for freeing resources does not naturally "fall" onto boundaries of functions or control structures.

Tristram answered 15/3, 2012 at 13:20 Comment(0)
E
16

Everyone else already covered correctly the scoping, RAII etc. possiblities, but since you mention an embedded environment, there is one further potential reason:

Maybe the developer doesn't trust this compiler's register allocation or wants to explicitly control the stack frame size by limiting the number of automatic variables in scope at once.

Here isInit will likely be on the stack:

{
   bool isInit;
   (void)isStillInInitMode(&isInit);
   if (isInit) {
     return isInit;
   }
}

If you take out the curly braces, space for isInit may be reserved in the stack frame even after it could potentially be reused: if there are lots of automatic variables with similarly localized scope, and your stack size is limited, that could be a problem.

Similarly, if your variable is allocated to a register, going out of scope should provide a strong hint that register is now available for re-use. You'd have to look at the assembler generated with and without the braces to figure out if this makes a real difference (and profile it - or watch for stack overflow - to see if this difference really matters).

Ethel answered 15/3, 2012 at 17:3 Comment(1)
+1 good point, although I'm fairly sure modern compilers get this right without intervention. (IIRC - for non-embedded compilers at least - they ignored the 'register' keyword as far back as '99 because they could always do a better job than you could.)Tournai
N
15

This is really useful when using scoped locks in conjunction with critical sections in multithreaded programming. Your scoped lock initialised in the curly braces (usually the first command) will go out of scope at the end of the end of the block and so other threads will be able to run again.

Nazler answered 14/3, 2012 at 19:22 Comment(0)
T
13

I think others have covered scoping already, so I'll mention the unnecessary braces might also serve purpose in the development process. For example, suppose you are working on an optimization to an existing function. Toggling the optimization or tracing a bug to a particular sequence of statements is simple for the programmer -- see the comment prior to the braces:

// if (false) or if (0) 
{
   //experimental optimization  
}

This practice is useful in certain contexts like debugging, embedded devices, or personal code.

Tympanites answered 15/3, 2012 at 3:20 Comment(0)
V
10

I agree with ruakh. If you want a good explanation of the various levels of scope in C, check out this post:

Various Levels of Scope in C Application

In general, the use of "Block scope" is helpful if you want to just use a temporary variable that you don't have to keep track of for the lifetime of the function call. Additionally, some people use it so you can use the same variable name in multiple locations for convenience, though that's not generally a good idea. E.g.:

int unusedInt = 1;

int main(void) {
  int k;

  for(k = 0; k<10; k++) {
    int returnValue = myFunction(k);
    printf("returnValue (int) is: %d (k=%d)",returnValue,k);
  }

  for(k = 0; k<100; k++) {
    char returnValue = myCharacterFunction(k);
    printf("returnValue (char) is: %c  (k=%d)",returnValue,k);
  }

  return 0;
}

In this particular example, I have defined returnValue twice, but since it is just at block scope, instead of function scope (i.e., function scope would be, for example, declaring returnValue just after int main(void)), I don't get any compiler errors, as each block is oblivious to the temporary instance of returnValue declared.

I can't say that this is a good idea in general (i.e., you probably shouldn't reuse variable names repeatedly from block-to-block), but in general, it saves time and lets you avoid having to manage the value of returnValue across the entire function.

Finally, please note the scope of the variables used in my code sample:

int:  unusedInt:   File and global scope (if this were a static int, it would only be file scope)
int:  k:           Function scope
int:  returnValue: Block scope
char: returnValue: Block scope
Vern answered 14/3, 2012 at 14:54 Comment(1)
Busy question, man. I've never had 100 ups. What's so special about this question? Good link. C is more valuable than C++.Homemaker
P
6

So, why to use "unnecessary" curly braces?

  • For "Scoping" purposes (as mentioned above)
  • Making code more readable in a way (pretty much like using #pragma, or defining "sections" that can be visualized)
  • Because you can. Simple as that.

P.S. It's not BAD code; it's 100% valid. So, it's rather a matter of (uncommon) taste.

Paring answered 15/3, 2012 at 13:24 Comment(0)
V
5

After viewing the code in the edit, I can say that the unnecessary brackets are probably (in the original coders view) to be 100% clear what will happen during the if/then, even tho it is only one line now, it might be more lines later, and the brackets guarantee you wont make an error.

{
   bool isInit;
   (void)isStillInInitMode(&isInit);
   if (isInit) {
     return isInit;
   }
   return -1;
}

if the above was original, and removing "extras" woudl result in:

{
   bool isInit;
   (void)isStillInInitMode(&isInit);
   if (isInit) 
     return isInit;
   return -1;
}

then, a later modification might look like this:

{
   bool isInit;
   (void)isStillInInitMode(&isInit);
   if (isInit) 
     CallSomethingNewHere();
     return isInit;
   return -1;
}

and that, would of course, cause an issue, since now isInit would always be returned, regardless of the if/then.

Virtue answered 15/3, 2012 at 15:2 Comment(0)
C
4

Objects are automagically destroyed when they go out of scope...

Centralize answered 14/3, 2012 at 15:45 Comment(0)
C
4

Another example of usage is UI-related classes, especially Qt.

For example, you have some complicated UI and a lot of widgets, each of them got its own spacing, layout, etc. Instead of naming them space1, space2, spaceBetween, layout1, ... you can save yourself from non-descriptive names for variables that exist only in two-three lines of code.

Well, some might say that you should split it in methods, but creating 40 non-reusable methods doesn't look ok - so I decided to just add braces and comments before them, so it looks like logical block.

Example:

// Start video button
{
   <Here goes the code >
}
// Stop video button
{
   <...>
}
// Status label
{
   <...>
}

I can't say that's the best practice, but it's good one for legacy code.

Got these problems when a lot of people added their own components to UI and some methods became really massive, but it's not practical to create 40 onetime-usage methods inside class that already messed up.

Contrapose answered 21/12, 2016 at 23:21 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.