break in a case with return.. and for default

Question

My OCD makes me add "break" when writing case statements, even if they will not be executed. Consider the following code example:

switch(option) {
    case 1:
        a = 1;
        b = 7;
        break;
    case 2:
        a = 2;
        b = 4;
        return (-1);
        break;
    default:
        a = -1;
        break;
}

My two questions are:
For "case 2:", I don't really need the break, but is it a good idea to have it there anyway? For "default:". Is it purely OCD, or is there any real reason to have the break here?

Answer 1

I would consider the break after return to be bad form, you will get warnings about unreachable code on some compilers.

The break on your default case is completely appropriate, case fall through is a tool and should be especially marked when used.

Answer 2

Neither break does anything for you, but neither does harm.

Personally, I usually leave them out if I have a return - but I also try to avoid having multiple return points in a function if possible.

However, I do think the break in the default: case is good - for one reason: If you were to leave it out, and somebody added a new case after default:, the behavior would be different if they "forget" to add in a break.

Answer 3

I would put the break in to show that you do not intend to fall through to next case.

Answer 4

Regarding the comment that others have made that they leave the break in the default case in case someone comes by later and adds a case after it: Where I work, the coding standard says to always put the default as the last case; so in our situation, a break on that case is just redundant. (This is one case where I agree wholeheartedly with the company's coding standard, because with the default case always being the last one, you always know where to find it, even in a long switch-statement.)

As for breaks after returns, I tend to omit the break unless there are any execution paths that do not return, as I find it redundant. (My exception to this is, on the rare occasions when there are several execution paths in a case and I can't tell with a quick scan of the code whether or not they all return, then I'll leave the break in just to be safe.)

Answer 5

I agree with having a break in a final default case, and don't agree with breaks after returns. (A colleague does those and it hurts my eyes.)

I also indent switches so as to reduce proliferation of indent levels. :) i.e.:

switch(option) {
case 1:
    a = 1;
    b = 7;
    break;
case 2:
    a = 2;
    b = 4;
    return -1;
default:
    a = -1;
    break;
}

(I also think that, since the return statement is not a function, it isn't appropriate to enforce a superfluous style that makes it look like one.)

Answer 6

As others have pointed out, placing a break after a return or in the default case is mostly a matter of personal style.

When I don't have to follow any specific style rules, I prefer something like this:

    switch(foo){
         case 0:
             baz = 1;
             break;
         case 1:
             bar %= 2;
             return -1;
             /* NOTREACHED */
         case 2:
             bar = -1;
             return -2;
             /* NOTREACHED */
             break;
         default:
             break;
    }

Between cases 1 and 2, I tend to prefer 2. Even though the comment says NOTREACHED, comments can lie ( unintentionally of course ) when the code changes. I like the NOTREACHED comment since it can satisfy lint that you know what you are doing and serves as a reminder that you exiting the function early. The reasoning that placing a break after the return will mitigate errors if the return is deleted seem flawed to me. You are still going to get bogus behavior regardless if you fall through to the next case or you exit the switch and continue on as before.

Of course, if I can avoid it I would not return from a function within the body of a switch.