4

I see most developers using switch statements with breaks in each and every case. Is this usage of switch statements inappropriate since a simple set of if-else statements would suffice?

Is it OK for me to think that the only reason for you to use a switch is because two or more cases are related, so you intentionally want some statements to "fall through"?

An example in which I think it's OK to use a switch is the case in which you want to update the schema of a database; so if you are updating from version 1 to version 4, you want to make sure the schema goes through versions 2 and 3 first:

private void updateDatabase(int oldVersion){
   switch(oldVersion){
      case 1:
         updateToVersion2();
      case 2:
         updateToVersion3();
      case 3:
         updateToVersion4();
   }
}

A NOT OK example of a switch is where I use it in a video game to move a character based on the button pressed. I would have to break each case so that when pressing LEFT the character doesn't move in all directions:

private void onButtonPressed(Button button){
   switch(button){
      case LEFT:
         moveLeft();
         break;
      case RIGHT:
         moveRight();
         break;
      case UP:
         moveUp();
         break;
      case DOWN:
         moveDown();
         break;
    }
 }
5
  • 1
    Um, you know that C# requires a break (or other "jump" statement) to end a case, right? Jan 14, 2015 at 1:28
  • @RossPatterson My bad. Java, C++, JavaScript and most other languages then. Jan 14, 2015 at 1:33
  • 5
    The schema transition is a brilliant example of using a switch statement! Right on!
    – Brandon
    Jan 14, 2015 at 1:45
  • What is it about putting a break at the end of every case that makes it not OK? Your first example could be done with if statements, too.
    – Blrfl
    Jan 14, 2015 at 4:19
  • I tend to find that I'll use a switch case if I'm going to do more than one else if. For me it's all about readability so I also then go ahead and bury the switch statement in it's own method to ensure it's as clean as possible. There's no huge difference between the two really, I'd say which ever is more readable is what you should be using. Jan 14, 2015 at 9:15

4 Answers 4

7

No, switch statements are not generally used wrong.

They are mostly used for their intended use: enumerating action alternatives for a smallish set of possible input values. It's more readable then a long if/else chain, compilers can emit very efficient code if the checked values are reasonably contiguous, and it's easier to write error-checking compilers that warn you when you forget an alternative. All this is well and good. (Using switch when inheritance and dynamic dispatch would be preferable is a common mistake among beginners who don't understand OOP very well yet, but that is a higher-level issue.)

However, switch is defined badly in most languages.

Since by far the most common use case is to do something different for each case, it's annoying and clutterful having to write break after each case that isn't a fall-through. The switch statement would work much better if break were the default after a case body, and you had to order fall-through via an explicit fallthrough keyword (unless two cases are directly adjacent). That would remove a lot of unnecessary lines, and many, many people wouldn't have made subtle errors by forgetting a break.

However, it's definitely too late now to do anything about that; C does it that way, and none of its descendants was brave enough to change anything about it, so for the time being, switch will probably stay as it is.

1
9

Since if/else is similar to a switch and often interchangeable, your confusion is understandable.

Some languages, such as Python, don't even have a switch statement. Not surprisingly, when you Google for "python switch", the first result points to an alternative—a map. Not an if/else, but a map.

While you can use if/else every time instead of a switch, you should also understand that your colleagues are not wrong when they use a switch statement. Either they were simply told by a teacher or a mentor that they should use a switch when there are more than two branches, or they simply find it more elegant or readable, both terms being perfectly subjective.

Your first example, by the way, annoys me a lot. If I were a reviewer of your code, I would immediately flag it, because it's easy to overlook the lack of break statements, and so it looks like if oldVersion is 2, only updateToVersion3() will be executed, but not updateToVersion4(). Avoid writing code which is prone to errors. For instance, such syntax is forbidden in C# precisely in order to avoid mistakes which are so easily avoidable.

Your second example is a case where switch is perfectly fine. Although I would use instead either a map (mapping functions—or even lambda expressions, and not their results) or inheritance, I would switch to switch statement if I know that my code will be read by beginners who may not understand those two approaches.

If the style guidelines of your code base allow it, your second example may even be written this way:

switch (button){
    case LEFT: moveLeft(); break;
    case RIGHT: moveRight(); break;
    case UP: moveUp(); break;
    case DOWN: moveDown(); break;
}

or like this:

switch (button){
    case LEFT:  moveLeft();  break;
    case RIGHT: moveRight(); break;
    case UP:    moveUp();    break;
    case DOWN:  moveDown();  break;
}

making the code shorter and more readable (again, readability is subjective) than an if/elif/else variant.

An important aspect highlighted by J Trana is that in this second example, if/elif/else may be error prone. I can't easily find a more illustrative example where you can migrate to switch, but the following piece of code shows the idea. Are you able to immediately see why the following code will sometimes throw an exception at runtime (imagine it's 6 PM and you've spent three hours inspecting code)?

if (!fs.FileExists(fileFullPath)) {
    debug("The file appears to be missing. Check if the location is set in the options.");
}
if (fs.ReadFile(fileFullPath).Contains(textToFind)) {
    found = true;
    debug("The file contains the expected text.");
}
else {
    debug("The text was not found.");
}

Here again, in C#, the official style guide will prevent you from writing this error prone piece of code: it would require to add a line break before the second if, and the author of the code will immediately see that he wrote if instead of else if.

That's why, in order to reduce the risk of writing code which doesn't express clearly the intentions of the author and can easily be misread:

  • Avoid the missing breaks in switch statements, except in the case where the previous case contains no logic:

    switch (something)
    {
        case 1: // Not having a `break` is fine. The intention of the author is clear.
        case 2:
            hello();
            break;
    
        case 3:
            world();
            break;
    }
    
  • Avoid else if which may be confused with or, by mistake, replaced by, an if, unless the style guidelines are protecting you.

  • Use maps and inheritance when appropriate. In most cases, maps and inheritance are more appropriate than if/elif/else or switch. Longer if/elif/else or switch tend to be a good sign that they should be replaced by a map (often) or an inheritance (rarely) or should be completely refactored (often).

    The record I've seen so far is a switch statement containing approximately a thousand cases (written by an expert proud of his 15 years' professional experience as a lead developer).

  • Flatten hierarchies when possible. This code:

    if (n == a) {
        f1();
    }
    else {
        if (n == b) {
            f2();
        }
        else {
            f3();
        }
    }
    

    should immediately be refactored to:

    if (n == a) {
        f1();
    }
    else if (n == b) {
        f2();
    }
    else {
        f3();
    }
    

    or:

    switch (n) {
        case a:  f1(); break;
        case a:  f2(); break;
        default: f3();
    }
    
  • Beware of insane programmers. This piece of code is equivalent to the one in the previous point, if a, b, c and n are numbers (things would be different if those were calls to functions with side effects):

    if (!(n == a)) {
        if (n - b == 0 && !(n > b)) {
            f2();
        }
    }
    
    if (!(n == a) && n - b < 0 || b - n < 0) {
        f3();
    }
    
    if (n != a) {
        // Do nothing.
    }
    else {
        if (!(n > a) && ! (n < a)) {
            f1();
        }
    }
    
6
  • Speaking of error-prone code, am I the only that's accidentally used an if-if rather than an if-else-if in a chain of ifs and gotten burned by two blocks executing? The "NOT OK" use of cases with breaks (especially in C#) has probably saved me quite a few times because it usually implies mutual exclusion.
    – J Trana
    Jan 14, 2015 at 2:56
  • 1
    "it's easy to overlook the lack of break statements". The lack of break statements is the whole point; in my opinion it's not a flaw, it's what makes the use of the switch OK in the example. Jan 14, 2015 at 3:00
  • @JTrana: great point. I added it to the answer, although I can't find a great example where if/if/else can be replaced by a switch. Jan 14, 2015 at 4:31
  • 1
    @YasmaniLlanes: "The lack of break statements is the whole point": yes, I know. The problem is, it makes the code error prone, given that the error can easily be avoided. Jan 14, 2015 at 4:32
  • 1
    @Jules: good point. I think it depends on the culture of a particular programmer. Having spent the last 7 years working with C#, I can hardly imagine myself writing fallthrough cases in any language. Other persons with a different background may find it not only acceptable, but even natural. Jan 14, 2015 at 11:45
2

I see what you're saying, but this is ultimately not improper.

If it's just one or two elses, an if-else change will suffice. But if it grows to be more than that, I'd say switch statements can quickly turn into something that's just cleaner and less of an eyesore to look at. Or...depending on who you are personally, it could turn it into something that's even messier.

It's strictly cosmetic in the case where you always break out of everything, but saying that it's particularly bad is like saying that between /*...*/ comments and // comments, one is completely better than the other. If nothing's wrong with long if-else chains, then what's wrong with the more trivial switch statements? It's just cosmetic, in cases where this question applies, and you'll find people on each side of the fence on which looks neater. There is no huge convention over this.

But if something is wrong with switch statements where you do break out of everything, as opposed to just using if-else chains, then what is there to justify switch statements where you don't break out of everything? Take your first example (which I agree with Brandon, it was sharp!):

private void updateDatabase(int oldVersion){
    switch(oldVersion){
        case 1:
            updateToVersion2();
        case 2:
            updateToVersion3();
        case 3:
            updateToVersion4();
    }
}

Couldn't this just as easily have been written as the following:

private void updateDatabase(int oldVersion){
    if (oldVersion > 0 && oldVersion < 4){
        if (oldVersion < 3){
            if (oldVersion < 2){
                updateToVersion2();
            }
            updateToVersion3();
        }
        updateToVersion4();
    }
}

Is this better or worse cosmetically? It's a matter of opinion. Is this better or worse in terms of performance? Technically, depending on how the language is compiled, there's probably one extra int vs. int comparison that's been introduced, but is that really going to cause one microscopic fraction of the performance hit that using a high-level programming language will? Realistically, you've probably either already made sure that oldVersion is more than 0, or you would need to add a similar check to your function with the switch statement.

Some people even think this:

switch (true)
{
    case variable1:
        doSomething1();
        break;

    case variable2 && variable3:
        doSomething2();
        break;

    case !variable4 && variabl5:
        doSomething3();
        break;
        .
        .
        .
    default:
        doSomethingElse();
}

is ultimately cleaner code than:

if (variable1)
{
    doSomething1();
}
else if (variable2 && variable3)
{
    doSomething2();
}
else if (!variable4 && variable5)
{
    doSomething3();
.
.
.
}
else
{
    doSomethingElse();
}

(Notice the difference in where the curly braces were placed in the last two snippets; when people do it this way, which is the subject of an ongoing debate in certain languages, it will probably affect their disposition towards the cleanliness of switch statements.)

Nothing's wrong with the trivial break-out-of-everything use of switch statements, but an experienced programmer should be able to see what it is you hold against them.

6
  • Which language allows non-constant case labels? I don't know of any. Jan 14, 2015 at 8:50
  • @BartvanIngenSchenau JS, PHP, etc.
    – Hey
    Jan 14, 2015 at 9:44
  • @Hey: Thanks. It seems to be an unintended usecase as I had to really dig through the documentation to find examples of it. Jan 14, 2015 at 9:56
  • @BartvanIngenSchenau unfortunately it's more common "in the trenches" ;)
    – Hey
    Jan 14, 2015 at 9:59
  • @BartvanIngenSchenau I don't specifically remember ever using it myself, though it's probably happened once or twice. A high-ranking AS3 guy on Stack Exchange brought it up one time, I think on Stack Overflow, and then it's just been something in the back of my mind since. I tend to think it's cleaner than a long if-else chain, especially if you're dealing with fall-through cases, but not if it's a real short one. Jan 14, 2015 at 13:40
2

Some languages (Scala compiled to JVM, Haskell, Ocaml, etc...) have pattern matching which is superior to switch since its deconstruct data structures and bind variables (locally to the matching case).

For example, in Ocaml you might define a simplistic AST data type (a sum type or tagged union) for expressions as:

type expr = 
    Num of int
   | Sum of expr * expr
   | Diff of expr * expr
   | Prod of expr * expr
  (* etc... *)
;;

Then you would represent the AST of 2*(3+4) as

   Prod (Num 2, Sum (Num 3, Num 4))

and your recursive evaluation function is

let rec eval exp = 
  match exp with
   Num x -> x
   | Sum (el,er) -> (eval el) + (eval er)
   | Diff (el,er) -> (eval el) - (eval er)
   | Prod (el,er) -> (eval el) * (eval er)
   (* etc....*)

Notice that the matching variables el and er have their scope limited to each match clause, like lines above starting with | (and they are bound by the matching operation)

The compiler generally translates pattern matching into switch like constructs (e.g. indexed jumps) and field extractions.

Not the answer you're looking for? Browse other questions tagged or ask your own question.