Braces Are Good

From: SelfDocumentingCode / CodeFormatting

Braces are a stronger way to break code up than whitespaces.

But methods are better. cf. MethodsVsCodeFragments

Rationale:

When writing methods, sometimes a group of statements logically fits together to do one step of an action. Whitespace alone is not strong enough to distinguish the code fragment from the rest of the source.

Braces (or any way to begin and end a block) come to the rescue.

Not only do they visually distinguish one group of code from another, but they provide a scoping layer to preserve names and clean up objects.

Arguments:

"I like methods better."

Good choice. Typically, this is not a bad choice. Sometimes, it is useful to not create a separate helper method for a given step. In these cases, use braces.

"Braces mean another level of indenting. Code is getting squished to the righthand side of my monitor!"

Your method is likely far too complicated. Try breaking it up into helper methods instead.

Exceptions: You don't always need to break each step into its own subsection. Only when readability is getting low. Using helper methods may be useful, but sometimes they don't seem quite so appropriate.

It's very subjective.

Examples:

 int main( int argc, char **argv )
 {
char szModuleName[MAX_PATH];

// Extract the module name from argv[0]
{
 char *szLastSlash = strchr( argv[0], '\\' );
 strncpy( szModuleName, szLastSlash + 1 );

 char *szExtension = strchr( szModuleName, '.' );
 *szExtension = '\0';
}

...
 }

Maybe you could have passed argv[0] to a HelperFunction in this case, but it's unlikely you'll need to extract the module name again.

Note how the two char * variables fall out of scope and thus are unavailable to the rest of the program. This also prevents name collisions.

I HaveThisPattern. It is particularly useful for delimiting code that might be subject to ExtractMethod later. It's also useful as a way for retaining scoping behaviors while using InlineMethod. In C++ scoping behaviors can be very important because of the ResourceAllocationIsInitialization idiom. In that language leaving a block means (or can mean) releasing any resources used in that block. -- PhilGoodwin

This supports the argument that the braced code should be extracted to a helper method. Bracing is an excellent first step for localizing your data in preparation for extraction.

I use this for delimiting long blocks of debug code:

  doSomething();
  { //debug begins
System.out.println ("foo = " + foo);
System.out.println ("bar = " + bar);
for (int i = 0; i < items.length; i++)
System.out.println ("items [" + i + "] = " + items [i] );
  } //debug ends
  doSomethingElse();

this allows debug code to introduce variables that don't mess up the code you're trying to debug, and it makes it easy to figure out which code to remove after you fix the problem. -- anonymous

:Wow, this is way cool! EdPoor

Cooler still can be:

  if(debug){
//Do debug stuff when static or instance variable is set
  }

  if(debug()){
//Do debug stuff when protected member function is overridden
//in anonymous class 
  }

and even

  if(false&&debug||debug()){
//Do debug stuff when protected member function is overridden
//or static variable is set, but not for this compile
  }

Even when (you think) the problem is fixed you may not need to remove most such blocks as they should be optimised out by the compiler - DavidWright

I tend to use braces this way also. I would also suggest marking the beginning and end of your debug with #if defined() / #endif markers. I also usually add a remove date comment. This way I can leave the debug statements in for a couple of iterations without affecting production.

-- WayneMack

I like to start my blocks with something that explains the reason. For example:

  #define DEBUG_BLOCK if (_NO_DEBUG) {} else

  do_something1();
  DEBUG_BLOCK
  {
whatever();
  }
  do_something2();

Note that the block is the "else" clause, to avoid subtle bugs. An alternative is to create a macro whose arg is the block; but I find many editors aren't too good at highlighting that style. Wrapping debug blocks in #ifdef ... #endif is more visually distracting (and doesn't support the % key in vi). The DEBUG_BLOCK macro can be made more powerful by adding parameters. -- DaveWhipp

vi may not allow use of the % key to jump from #if to #endif, but vim certainly does.

I personally do not like this approach as it is using the CeePreprocessor to create new language constructs. There is nothing worse than to pick up code and have to decode a particular programmer's private syntax. C programmers will recognize an #ifdef or #if defined statement and know what it is. The same cannot be said of a DEBUG_BLOCK macro. --WayneMack

I HaveThisPattern, too. But I think I shouldn't. The blocked code by definition is of different purpose than the rest of the method. Therefore the method does two things. But the meanings of the things are not clearly communicated. Therefore the blocked code should be removed from the method, given a clear name, and sent from within. That I HaveThisPattern is due, I fear, to the fact that I'm lazy. -- RonJeffries

I generally think of code written in this style as having one foot out the door. It's all packed up and ready to go (or just arrived and not yet unpacked as the case may be). I just see it as a stable snapshot of a more dynamic process. If the code wants to stay it'll start making itself more comfortable. If it wants to go it'll probably see a place that it would rather be in short order. Usually I can just wait for more information and do the right thing when the time comes for doing rather than waiting.

In some cases, however, it really shows that the current method must make some shift in context and come back in order to do its work. This shows up most clearly when I'm doing concurrent programming. I'll use ResourceAllocationIsInitialization to manage the lifetime of various locks and then construct the objects that claim the locks I need right at the top of a brace delimited block. The result is that it is very clear which locks are held and for how long within the body of the code. This can be done in Java by applying the synchronized keyword to a block. -- PhilGoodwin

I first saw this pattern in a talk by Rob Whitty in the early 1970's. "You need a way to comment a block of code not just put a comment in the middle of code", or something like that. In other words.... a time tested pattern.

-- DickBotting

Maybe you could have passed argv[0] to a helper function in this case, but it's unlikely you'll need to extract the module name again.

Who cares? If you are unlikely to do it often, then it's not exactly a performance-critical piece of code. Put in its own function, give it a meaningful name, and forget both the redundant comment and the weird "I've added some syntax here, I'll let you guess why" braces:

 int main( int argc, char **argv )
 {
char *szModuleName = extractModuleName(argv[0]);

...
 }

 static char *extractModuleName(char *argv0) {
static char szModuleName[MAXPATHNAMELEN];// static?  How lazy. Someone who understands std:string should fix it. What if this is C code, not C++ code?
strncpy(szModuleName, rindex(argv0, '/') + 1);

strchr(szModuleName, '.')[0] = '\0';
return szModuleName;
 }

A better argument than the reuse argument is that szModuleName will certainly have additional methods related to it. By extracting the methods related to this data element into a common C module or a common C++ class, it makes it easier to modify the data element and methods later on. For example, since this is a call into Main(), there is no reason to create additional storage for the module name. One can merely hold on the argument pointer and modify the [assumed] Compare() method later to find the last slash and stop at the last period. It is often best to defer operations until needed, but this is difficult to do if the methods operating on a data element are spread throughout the code. --WayneMack

Indeed, methods are better. But sometimes, sometimes it's nicer to scope. Especially in C++ to call the destructor (for auto-locking synchronization objects as mentioned above), or to wrap for loops to protect against broken compilers who place the loop variable in the outer scope, or just because creating a new method seems like overkill. Such as when you are using IntermediateValues while calculating a formula.

-- SunirShah

Commented braces are good, but there was an experiment done along time ago that suggests that "naked braces" tend to generate more logic mistakes than ones with comments.... Whatever I tend (in C/C++) to comment close braces with some clue as to what I'm closing, so you might find at the end of a piece of code:

 }//while not eof
  filep.close();
}//load(string filename)
 };//MyDumbClass?

-- DickBotting

Some reasons not to do ClosingBraceComments?:

Your logic is too complex if you need them... refactor it!
Gets in the way of reading the code.
The comments might be *wrong*. ("...are usually wrong...")
Indentation serves as a reasonable alternative

Dead right. In the example above, there are three lines consisting almost entirely of comment and one line that actually does something (filep.close();). It's not easy to see what's going on, because the actual code gets lost in the comments. This would be less bad with a syntax-colouring editor, but why suffer unnecessary pain?

One approach that I suggest to those who just can't give up commenting their close-braces: put the comments somewhere over to the right, out of the way of the code.

Thus:

 int foo(void) {
if (!boring()) {
while (not_done()) {
do_something();
}// while not done
}// if not boring
 }  // end of int foo(void)

I still think it's better to learn to do without, though... -- GarethMcCaughan

Why on earth would anyone use commented braces? How could you have that much fear of creating a method? After all, isn't a method just a comment with braces, and compiler enforcement?

Why do:

   //do stuff with foo
   {
       stuff....
   }

rather than:

   doStuffWithFoo()
   {
       stuff....
   }

am I missing something?

Comments on closing braces can be useful when the brace encloses a region of code which is too big to see all at once. Although functions shouldn't get that big, it can happen with classes or namespaces, or with #endif. Some people like simple style rules. If some braces need comments and some don't, rather than let the programmer decide on a case by case basis, they mandate that all braces have comments, because it's a simpler rule.

I HaveThisPattern in perl, particularly for scripts that do a sequence of semi-independent actions. The braces allow me to declare variables local to the code block, and at the same time still see the sequence of the whole script. As suggested above, they are also a good hint that I can refactor the block into a subroutine if I need to do the same thing again. I consider it a manifestation of YouArentGonnaNeedIt: why give a code block a name, if I don't need one yet?

The format the code is written in (spaces, line breaks, ...) has to correlate with the structure the compiler or interpeter understands. So they depend on each other. Using both is redundant and goes against DontRepeatYourself. It increases the danger of bugs.