Switch statement with returns — code correctness

Question

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

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 2

I think the *break*s are there for a purpose. It is to keep the 'ideology' of programming alive. If we are to just 'program' our code without logical coherence perhaps it would be readable to you now, but try tomorrow. Try explaining it to your boss. Try running it on Windows 3030.

Bleah, the idea is very simple:

---

Switch ( Algorithm )
{

 case 1:
 {
   Call_911;
   Jump;
 }**break**;
 case 2:
 {
   Call Samantha_28;
   Forget;
 }**break**;
 case 3:
 {
   Call it_a_day;
 }**break**;

Return thinkAboutIt?1:return 0;

void Samantha_28(int oBed)
{
   LONG way_from_right;
   SHORT Forget_is_my_job;
   LONG JMP_is_for_assembly;
   LONG assembly_I_work_for_cops;

   BOOL allOfTheAbove;

   int Elligence_says_anyways_thinkAboutIt_**break**_if_we_code_like_this_we_d_be_monkeys;

}
// Sometimes Programming is supposed to convey the meaning and the essence of the task at hand. It is // there to serve a purpose and to keep it alive. While you are not looking, your program is doing  // its thing. Do you trust it?
// This is how you can...
// ----------
// **Break**; Please, take a **Break**;

/* Just a minor question though. How much coffee have you had while reading the above? I.T. Breaks the system sometimes */

Answer 3

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 4

Exit code at one point. That provides better readability to code. Adding return statements (Multiple exits) in between will make debugging difficult .

Answer 5

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

Answer 6

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?"