CODEWITHSUNDEEP
c++syntax
jackhab
jackhab

Reputation: 17708

where to put break in switch/case statement with blocks

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

case FOO:  // 'break' inside
  {
    int i;
    doStuff();
    break;
  }

case BAR: // 'break' outside
  {
    int i;
    doStuff();
  }
  break;

Thanks.

Upvotes: 25

Views: 15496

Answers (15)

wjl
wjl

Reputation: 7775

This is an old question, but I think there is something important that can be added here: style matters not just for aesthetics, but can actually make a difference in helping to mistake-proof real software written by real, fallible, humans.

Although where to put the break statement is certainly a matter of style, not a requirement of the language, there is one convention I haven't seen mentioned here that if followed makes it much easier to read, write, and review switch statements while ensuring that there is never any unintended fall-through.

The style looks like this:

switch(variable) {
    break; case 1 : statement;
    break; case 2 : {
       code in a block;
    }
    break; case 3 : other statement;
    /****/ case 4 : fall-through here on purpose.
    break; default: default behavior;
}

Although you could argue this is just a minor stylistic variation, in my experience (with over 20 years of using this style in critical embedded system code) its much easier to both write, review and maintain this kind of code. The biggest reason is because it changes the conceptual thought model.

Instead of thinking: "in this condition, do a bunch of stuff which I have to think hard about ... oh yeah, and then break or fall through", it forces the thought process to be: "this is either a brand-new condition, or a fall through from the previous case; now, in this condition, do a bunch of stuff which I can think hard about, and then move on."

Upvotes: 4

LastBlow
LastBlow

Reputation: 707

My take... See here below for an example.

Indent all cases of a switch and have the closing bracket of the switch under the keyword switch, just like one would do in an if statement. Whenever needed, same rule for each case statement: open brackets after the semi-column following the case statement and close them under the keyword case, then indent the contents of each of the case statements including the keyword break.

Keep it consistent (indentation and brackets placements) and short (no more than 5-10 lines of code per case statement. Complex projects have failed because of badly indented switch statements with too much code in them.

    switch (number) {

        case 1:
            logFileStderr(VERBOSE, "MESSAGE: switch without brackets...\n");
            break;

        case 2: {
            logFileStderr(VERBOSE, "MESSAGE: switch with brackets...\n");
            break;
        }

        default:
            logFileStderr(VERBOSE, "WARNING: Unknown option...\n");
            break;
    }

Upvotes: 0

Okken
Okken

Reputation: 2796

Real Answer: Compiler doesn't care. It's a matter of preference.

I put them inside, like the google style guide.

If using braces, I like the opening brace to be on the same column as the closing brace (which differs from google).

switch (var) {
  case 0: 
  {  
    ...      
    break;
  }
  case 1: 
  {
    ...
    break;
  }
  default: 
    assert(false);
}

Upvotes: 0

ChrisBD
ChrisBD

Reputation: 9209

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.

Upvotes: 6

Daniel
Daniel

Reputation:

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;
}

Upvotes: 1

William Pursell
William Pursell

Reputation: 212434

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.

Upvotes: 0

xtofl
xtofl

Reputation: 41519

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;
}

Upvotes: 4

Kirill V. Lyadvinsky
Kirill V. Lyadvinsky

Reputation: 99635

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;
    }

Upvotes: 0

Xetius
Xetius

Reputation: 46864

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.

Upvotes: 0

Scott Langham
Scott Langham

Reputation: 60391

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.

Upvotes: 1

ur.
ur.

Reputation: 2947

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.

Upvotes: 1

Greg Hewgill
Greg Hewgill

Reputation: 993971

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.

Upvotes: 4

Indy9000
Indy9000

Reputation: 8881

It's a matter of style.

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

Upvotes: 27

avakar
avakar

Reputation: 32645

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

Upvotes: 16

Jovan
Jovan

Reputation: 91

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.

Upvotes: 2

Related Questions