switch statement with returns -- code correctness

Question

Hi, let's say I have code in C with approximately this structure:

switch (something)
{
case 0:
  return "blah";
  break;

case 1:
case 4:
  return "foo";
  break;

case 2:
case 3:
  return "bar";
  break;

default:
  return "foobar";
  break;
}

Now obviously, the "break"s are not necessary for the code to run correctly, but it sort of looks like bad practice if I don't put them there to me.

What do you think? Is it fine to remove them? Or would you keep them for increased "correctness"?

Answer 1

Keep the breaks - you're less likely to run into trouble if/when you edit the code later if the breaks are already in place.

Having said that, it's considered by many (including me) to be bad practice to return from the middle of a function. Ideally a function should have one entry point and one exit point.

Answer 2

Remove the break statements. They aren't needed and perhaps some compilers will issue "unreachable code" warnings.

Answer 3

I would say remove them and define a default: branch.

Answer 4

Remove them. It's idiomatic to return from case statements, and it's "unreachable code" noise otherwise.

Answer 5

I would remove them. In my book, dead code like that should be considered errors because it makes you do a double-take and ask yourself "How would I ever execute that line?"

Answer 6

I'd normally write the code without them. IMO, dead code tends to indicate sloppiness and/or lack of understanding.

Of course, I'd also consider something like:

char const *rets[] = {"blah", "foo", "bar"};

return rets[something];

Edit: even with the edited post, this general idea can work fine:

char const *rets[] = { "blah", "foo", "bar", "bar", "foo"};

if ((unsigned)something < 5)
    return rets[something]
return "foobar";

At some point, especially if the input values are sparse (e.g., 1, 100, 1000 and 10000), you want a sparse array instead. You can implement that as either a tree or a map reasonably well (though, of course, a switch still works in this case as well).

Answer 7

Wouldn't it be better to have an array with

arr[0] = "blah"
arr[1] = "foo"
arr[2] = "bar"

and do return arr[something];?

If it's about the practice in general, you should keep the break statements in the switch. In the event that you don't need return statements in the future, it lessens the chance it will fall through to the next case.

Answer 8

I would take a different tack entirely. Don't RETURN in the middle of the method/function. Instead, just put the return value in a local variable and send it at the end.

Personally, I find the following to be more readable:

String result = "";

switch (something) {
case 0:
  result = "blah";
  break;
case 1:
  result = "foo";
  break;
}

return result;

Answer 9

Personally I would remove the returns and keep the breaks. I would use the switch statement to assign a value to a variable. Then return that variable after the switch statement.

Though this is an arguable point I've always felt that good design and encapsulation means one way in and one way out. It is much easier to guarantee the logic and you don't accidentally miss cleanup code based on the cyclomatic complexity of your function.

One exception: Returning early is okay if a bad parameter is detected at the beginning of a function--before any resources are acquired.

Answer 10

I say remove them. If your code is so unreadable that you need to stick breaks in there 'to be on the safe side', you should reconsider your coding style :)

Also I've always prefered not to mix breaks and returns in the switch statement, but rather stick with one of them.

Answer 11

For "correctness", single entry, single exit blocks are a good idea. At least they were when I did my computer science degree. So I would probably declare a variable, assign to it in the switch and return once at the end of the function

Answer 12

Interesting. The consensus from most of these answers seems to be that the redundant break statement is unnecessary clutter. On the other hand, I read the break statement in a switch as the 'closing' of a case. case blocks that don't end in a break tend to jump out at me as potential fall though bugs.

I know that that's not how it is when there's a return instead of a break, but that's how my eyes 'read' the case blocks in a switch, so I personally would prefer that each case be paired with a break. But many compilers do complain about the break after a return being superfluous/unreachable, and apparently I seem to be in the minority anyway.

So get rid of the break following a return.

NB: all of this is ignoring whether violating the single entry/exit rule is a good idea or not. As far as that goes, I have an opinion that unfortunately changes depending on the circumstances...

Answer 13

What do you think? Is it fine to remove them? Or would you keep them for increased "correctness"?

It is fine to remove them. Using return is exactly the scenario where break should not be used.

Answer 14

I personally tend to lose the breaks. Possibly one source of this habit is from programming window procedures for Windows apps:

LRESULT WindowProc (HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
{
    switch (uMsg)
    {
        case WM_SIZE:
            return sizeHandler (...);
        case WM_DESTORY:
            return destroyHandler (...);
        ...
    }

    return DefWindowProc(hwnd, uMsg, wParam, lParam);
}

I personally find this approach a lot simpler, succinct and flexible than declaring a return variable set by each handler, then returning it at the end. Given this approach, the breaks are redundant and therefore should go - they serve no useful purpose (syntactically or IMO visually) and only bloat the code.