where to put break in switch/case statement with blocks

Question

When I use braces around case code block in C++ to localize variables should I put break inside or outside the block?

Thanks.

Answer 1

It really depends on the logic of your code and how it uses braces, but for the sake of a proper answer, if you put one inside, try to put all of them inside.

Answer 2

You put it wherever you like. Make sure you stay consistent throughout the entire project. (Personally, I put it outside.)

Answer 3

It's a matter of style.

I would put break outside the closing brace just to make it more readable.

Answer 4

I usually put the break inside the braces, like this:

switch (foo) {
    case bar: {
        int x = 5;
        printf("%d\n", x);
        break;
    }
    case baz: {
        // ...
        break;
    }
}

However, since it's my rule I'm free to break it (no pun intended) whenever I want.

Answer 5

On my opinion you should avoid local variables and blocks in switch statements. Also you should avoid long, or complex or even cascaded switch statements anyway. But no rule without an exception ... I prefer to write the break statement after the block.

Answer 6

It should appear after.

For example:

switch(value)
{
   case 0:
   {
   // this ...
   // that ...
   // and the other ...
   }
   break;
}

Edited text below

This mostly to improve readability and maintainability here's an example.

switch (value)
{
   case 0:
    // Do this...
    // Do that...
    break;
   case 1:
    //and the other...
   break;
}

and

switch (value)
{
   case 0:
    // Do this...
    // Do that...
    if (ObjectWithinScope.Type == Fault)
    break;
   case 1:
    //and the other...
   break;
}

Now compare with

switch (value)
{
   case 0:
   {
    // Do this...
    // Do that...
   }
   break;
   case 1:
    //and the other...
   break;
}

and

   switch (value)
    {
       case 0:
       {
        // Do this...
        // Do that...
        if (ObjectWithinScope.Type == Fault)
        break;
       }
       case 1:
       {
        //and the other...           
       }
       break;
    }

When you start to come across cases of nested switch statements it can get very confusing indeed.

Just a pointer.

Now some of you are still wondering what I am getting at. Here it is. A piece of legacy code stopped working and noone could work out why. It all boiled down to a piece of code structured like the following:

   switch (value)
    {
       case 0:
       {
        // Do this...
        // Do that...
        if (ObjectWithinScope.Type == Fault)
        break;
       }
       case 1:
       {
        //and the other...           
       }
       break;
    }

It took a long time to pin down this code, but on checking the change logs it was originally as followws:

   switch (value)
    {
       case 0:
       {
        // Do this...
        // Do that...
        if (ObjectWithinScope.Type == Fault)
            // *** A line of code was here ***
        break;
       }
       case 1:
       {
        //and the other...           
       }
       break;
    }

Admittedly the original code wasn't consistant with itself, but by having the break within the curly brackets the code compiled when the one line of code was deleted by accident. If the break had been outside of the brackets then it wouldn't have.

Answer 7

It doesn't really matter as long as you and your team do the same thing consistently. Even then, it's not a big issue if different team members do it differently.

I personally prefer after. The reasoning is that it gives some separation between the machinery of the switch statement (jumping to, executing stuff and getting out), and the code within the braces which is purely involved with the 'doing' of the case.

eg:

switch( value )
{
    case 0:
        {
            // code here
        }
        break;

    default:
        {
            assert(!"unhandled value in switch");
        }
        break;
}

I only use {} for a case if it needs local variables, but if I use {} for any case, I put them on all cases.

I usually always define a default case to assert if there is any unexpected value. It's amazing how often one of these asserts fires to remind you of a missing case.

Answer 8

It is a matter for style, but I put it after as I understand the definition as:

switch (variable)
{
    case expression:
     statement;
     break;
    default:
     statement;
     break;
}

where statement is either a single command or a block. The break is separate to this statement or block. And yes, I do add a break after the default although it is superfluous. I also ALWAYS put braces around the statement. Too many times have I added one more statement to only to break the scope. And I add break to the default as I have changed default: to case expression: and added something after it. Defensive coding is your friend.

However, I can only find documentation for the actual definition via Microsoft as:

selection-statement:
    switch ( expression ) statement

labeled-statement:
    case constant-expression : statement
    default : statement

Which would indicate that it should be inside.

However, I think outside is clearer from a readers perspective, but it is certainly subjective.

Answer 9

Since standard does not limit you in choosing position for break statement you could choose whatever you like. Personally I'm using the following style:

 switch ( some_var ) {
     case 1: {
      // lot of code here
     } break;
     case 2: /* one call */ break;
     case 3: /* one call */ break;
     case 4: {
      // lot of code here again
     } break;
    }

Answer 10

Everyone agrees that we want a clearly recognizable distinction between the switch/case... machinery and the actual actions performed in each case.

Therefor, unless there's really almost nothing going on in each case (simple assignment or so), I suggest to use the switch as a mere dispatcher, and delegate the 'real' things to helper functions. That automatically solves the issue of case-local variables, and obsolesces the need for braces altogether.

switch( operation ) {
  case cnegation: r = -value; break;
  case cinversion: r = 1./r; break;
  case cfaculty: { 
    double r = value; 
    while( value != 1 ) 
      r *= --value; 
  }
  break;
}

Should then become

switch( operation ) {
  case cnegation : r = negate (value) ; break;
  case cinversion: r = invert (value) ; break;
  case cfaculty  : r = faculty(value) ; break;
}

Answer 11

There is a lot to be said for the style of only using one statement per case, and never using braces. For example:

switch( cond ) {
default: foo(); break;
case 0: bar(); break;
case 1: baz(); break;
}

Using this style, your question is moot.

Answer 12

I don't like putting any sort of brackets in a switch statement. Personally if it's a complex operation, I like putting it in a function. There's nothing more annoying than seeing a switch statement where between each "case" there are hundreds of lines of code and on top of that some of them are repeated in various cases which makes maintenance impossible.

For example:

switch(blah)
{
 case 1:
  // do one thing
  break;

 case 2:
  doManyThings();
  break;

 default:  
  // something
  break;
}