Is it bad practice to have a switch case in a switch case?

Question

Is it bad practice to have a switch case in a switch case? If so, what are alternatives? I don't really want to use if/else if if I don't need.

Instead of doing some like:

if((this == 1) && (that == 1)){ //something }
else if((this == 1) && (that == 2)){ //something }
else if((this == 2) && (that == 3)){ //something }

I was thinking along the lines of:

switch(this){
    case 1:
        switch(that){
            case 1:
                // something
            break;
            ....
        }
    break;
    ....
}

It just looks really wrong to me. Not wrong in syntax but wrong in terms of proper practice.

Answer 1

It's bad practise to have massive functions that do lots of different things. If you have a switch case in a switch case, that suggests your function is probably too big and you should consider splitting it up into smaller easier-to-understand chunks.

But there are no hard-and-fast rules; it all depends on the exact scenario.

Answer 2

Avoid following approach

switch(id)
{
    case 1:
    {
        switch (subid)
        {
            case 4:
                // nested code
        }
    }
    case 2:
         // some code
}

The preferable approach and improve your code by moving nested section into a method

switch(id)
{
    case 1:
        SubSwtich(subid);
        break;
    case 2:
         // some code
}

function SubSwtich(int subid)
{
        switch (subid)
        {
            case 4:
                // nested code
        }
}

Answer 3

i would consider it a bad pratice. in most cases this is unreadable.

you can extract the "sub" switch-cases to methods.

Answer 4

If it makes your code harder to read, then it's bad practice.

Whether that's the case in your particular example is up to you to decide, but in general I'd say it probably would be the case.

You asked for alternatives...

1. Extract some of your code into a sub-function. Eg:

   case 'foo' :
       myVar = 'blah';
       break;
   case 'bar' :
       myVar = secondLevelFunction();
       break;
   

   Here, the secondLevelFunction() contains the additional switch() statement where each case returns a value for myVar.

2. Use array mapping. Eg:

   var mapper = {'foo':'blah', 'bar':'bibble', etc};
   
   //now, instead of a big switch(input) { .... } block that sets myVar
   //for each option, you can just set it directly in a single line, like so:
   var myVar = mapper[input];
   

In addition, if you're looking for concrete measures of code quality, you should learn about Cyclomatic Complexity. This is a measure of how complex a function is. The measurement is taken by looking at how many "decision points" the function has. Each case, if, loop, etc is a "decision point". The more you have, the more complex your function.

Cyclomatic Complexity is stronly linked to code quality and good coding practices, so if your function has a high CC score (which it probably will do if it has multiple nested switch blocks), then it is a sign of poor code quality. Both the alternative solutions I described above can help with this. I'll leave it to you to read up more on CC.

Obviously the alternative solutions would need to be adapted to your needs, but hopefully they give you some ideas.

Answer 5

Bad practice? No! A potential pain when it comes to troubleshooting, probably! See what you can do about turning all the 'options' into something more organized and commonly connected. Maybe even write your own function using method overloading determined by the number of parameters being sent.

Take a look at this SO post and see if it gives you some ideas.

Answer 6

You should break your code in differ routines where depending upon the switch statement, and in the routine you can continue to use switch case as well. Just like following.

private void Switch(int value)
    {

        switch (value)
        {
            case 1:
                SwitchNest(value);
                break;
            case 2:
                break;
        }
    }

    private void SwitchNest(int value)
    { 
        switch (value)
        {
            case 1:
                SwitchOtherMethod(value);
                break;
            case 2:
                break;
        }
    }