C++ switch statement won't work

Question

I'm currently in a C++ course and to learn about switches our instructor wanted us to edit the body of some openGL 3D code. We were supposed to change it from a if then else to a switch. I did that, but now my code won't run. Here is the code:

void key(unsigned char k, int x, int y)
{
    k = tolower(k);

    switch(k)
    {
             case 'a' : b_animate = !b_animate;
                        if (b_animate)
                        {
                           glutTimerFunc(33, myTimer, 1);
                        }
                        break;
             case 'h' : b_showHints = !b_showHints;
                        glutPostRedisplay();
                        break;
             case 'f' : toggleFullScreen();
                        break;
             case 'o' : gi_projection_type = ORTHO_3D;
                        myReshape(g_windowWidth, g_windowHeight);
                        glutPostRedisplay();
                        break;
             case 'p' : gi_projection_type = PERSPECTIVE;
                        myReshape(g_windowWidth, g_windowHeight);
                        glutPostRedisplay();
                        break;
             case '1' :
             case '2' :
             case '3' :
             case '4' :
             case '5' : 
             case '6' :
             case '7' : 
             case '8' :
             case '9' : double alpha = (k - '0') * 0.1;
                        if (alpha == 0)
                        alpha = 1.0;
                        setAlphaChannel(alpha);   
                        glutPostRedisplay();     
                        break; 
             case '+' : per_angle += 2;
                        myReshape(g_windowWidth, g_windowHeight);
                        glutPostRedisplay();
                        break;
             case '-' : per_angle -= 2;
                        myReshape(g_windowWidth, g_windowHeight);
                        glutPostRedisplay();
                        break;
             case 'm' : printModelViewMatrix();
                        break;
             case 'w' : b_wireFrame = !b_wireFrame;
                        glutPostRedisplay();
                        break;
             case 'q' : b_useOpenGLtransform = !b_useOpenGLtransform;
                        glutPostRedisplay();
                        break;
             case 'n' : g_model = (g_model + 1)%8;
                        glutPostRedisplay();
                        break; 
             case 't' : b_texture = !b_texture;
                        glutPostRedisplay();
                        break;

             case 'l' : b_lighting = !b_lighting;
                        glutPostRedisplay();
                        break;

    }

Here is the original code (sorry it got screwed up a little bit, I was copying and pasting into a comment):

   if(k == 'a'){
   b_animate = !b_animate;
   if (b_animate){
       glutTimerFunc(33, myTimer, 1);

   }else if (k == 'h'){
    b_showHints = !b_showHints;
    glutPostRedisplay();

     }else if(k == 'f'){
     toggleFullScreen();

     }else if (k == 'o'){
         gi_projection_type = ORTHO_3D;
         myReshape(g_windowWidth, g_windowHeight);
         glutPostRedisplay();
   }else if (k == 'p'){
        gi_projection_type = PERSPECTIVE;
         myReshape(g_windowWidth, g_windowHeight);
         glutPostRedisplay();
    }else if (k >= '0' && k <= '9'){
    double alpha = (k - '0') * 0.1;
    if (alpha == 0)
       alpha = 1.0;
    setAlphaChannel(alpha);   
    glutPostRedisplay();     
             }else if (k == '+'){
      per_angle += 2;
      myReshape(g_windowWidth, g_windowHeight);
      glutPostRedisplay();

           }else if (k == '-'){
      per_angle -= 2;
      myReshape(g_windowWidth, g_windowHeight);
      glutPostRedisplay();

      }else if (k == 'm'){
       printModelViewMatrix();

       }else if (k == 'w'){
      b_wireFrame = !b_wireFrame;
      glutPostRedisplay();

      }else if (k == 'q'){
      b_useOpenGLtransform = !b_useOpenGLtransform;
      glutPostRedisplay();

       }else if (k == 'n'){
      g_model = (g_model + 1)%8;
      glutPostRedisplay();

      }else if (k == 't'){
      b_texture = !b_texture;
      glutPostRedisplay();

      }else if (k == 'l'){
      b_lighting = !b_lighting;
      glutPostRedisplay();
         }
         }    
}

The if then else statement will still run. The errors I'm getting with the switch are: syntax error before "double"

'alpha' undeclared (first use in this function)

Declare the `double alpha` before the switch statement. See http://stackoverflow.com/questions/1231198/declaring-variables-inside-a-switch-statement — Senti Bachcha, Nov 03 '13 at 20:51
"but now my code won't run" - whatever does that mean? Be specific. If you don't, you're basically being lazy or asking us to speculate — sehe, Nov 03 '13 at 20:53

Aleph · Answer 1 · 2013-11-04T16:17:12.663

You cannot declare variables inside a switch statement, unless you put them in a block (surround with {}) :

switch(k)
{
         case 'a' : b_animate = !b_animate;
                    if (b_animate)
                    {
                       glutTimerFunc(33, myTimer, 1);
                    }
                    break;
         case 'h' : b_showHints = !b_showHints;
                    glutPostRedisplay();
                    break;
         case 'f' : toggleFullScreen();
                    break;
         case 'o' : gi_projection_type = ORTHO_3D;
                    myReshape(g_windowWidth, g_windowHeight);
                    glutPostRedisplay();
                    break;
         case 'p' : gi_projection_type = PERSPECTIVE;
                    myReshape(g_windowWidth, g_windowHeight);
                    glutPostRedisplay();
                    break;
         case '1' :
         case '2' :
         case '3' :
         case '4' :
         case '5' : 
         case '6' :
         case '7' : 
         case '8' :
         case '9' : {                               // Note: begin of block
                    double alpha = (k - '0') * 0.1;
                    if (alpha == 0)
                    alpha = 1.0;
                    setAlphaChannel(alpha);   
                    glutPostRedisplay();     
                    break;
         }                                          // Note: end of block
         case '+' : per_angle += 2;
                    myReshape(g_windowWidth, g_windowHeight);
                    glutPostRedisplay();
                    break;
         case '-' : per_angle -= 2;
                    myReshape(g_windowWidth, g_windowHeight);
                    glutPostRedisplay();
                    break;
         case 'm' : printModelViewMatrix();
                    break;
         case 'w' : b_wireFrame = !b_wireFrame;
                    glutPostRedisplay();
                    break;
         case 'q' : b_useOpenGLtransform = !b_useOpenGLtransform;
                    glutPostRedisplay();
                    break;
         case 'n' : g_model = (g_model + 1)%8;
                    glutPostRedisplay();
                    break; 
         case 't' : b_texture = !b_texture;
                    glutPostRedisplay();
                    break;

         case 'l' : b_lighting = !b_lighting;
                    glutPostRedisplay();
                    break;

}

Alternatively, declare alpha before the switch statement.

Ivan Aksamentov - Drop · Answer 2 · 2013-11-03T21:36:57.853

You've become a victim of "all in one" function design. You function's body is unreadable mess.

Of course you can dig into this mess as much as necessary, until you fix it. But better re-think overall design. Split it to smaller functions, one for each key:

case 'a' : OnKeyA(); break;
case 'h' : OnKeyH(x, y, z); break
case 'f' : OnKeyF(x); break;

....

void OnKeyA()
{
   b_animate = !b_animate;
   if (b_animate)
   {
       glutTimerFunc(33, myTimer, 1);
   }
}

Isn't it much cleaner? Case statements become one-liners and all logic goes to functions. You will never get lost in curly braces.

Tell your instructor about "Separation of concerns" and "Single responsibility principle", he will be impressed =)

Happy coding!

score 1 · Answer 3 · answered Nov 03 '13 at 20:52

Whenever you need to declare variables inside a case, put it between curly brackets.-

case '9' : { 
    double alpha = (k - '0') * 0.1;
    if (alpha == 0)
        alpha = 1.0;
    setAlphaChannel(alpha);   
    glutPostRedisplay();     
    break; 
}

score 0 · Answer 4 · answered Nov 03 '13 at 20:53

you should wrap case handlers in {} like

         case '9' :
                 {
                    double alpha = (k - '0') * 0.1;
                    if (alpha == 0)
                    alpha = 1.0;
                    setAlphaChannel(alpha);   
                    glutPostRedisplay();     
                    break; 
                 }

because it is possible to declare new variables only within scope inside switch-case construction

score 0 · Answer 5 · answered Nov 03 '13 at 20:53

You need to add a scope for the double:

     case '9' : {
                   double alpha = (k - '0') * 0.1;
                   if (alpha == 0)
                      alpha = 1.0;
                   setAlphaChannel(alpha);   
                   glutPostRedisplay();     
                }
                break;

and you need to add

     case '0':

to the list before it! (You accidentially only had '1' to '9')

C++ switch statement won't work

5 Answers5