Why can't I have a variable in switch-case statement?

Question

Here is my code:

bool Character::keyPress(char c)
{
    switch(c)
    {
        case up_key:
            move(0, -1);
            break;

        case down_key:
            move(0, 1);
            break;

        case left_key:
            move(-1, 0);
            break;

        case right_key:
            move(1,0);
            break;

        default:
            return false;
    }

    return true;
}

And the compiler complains:

error C2051: case expression not constant
error C2051: case expression not constant
error C2051: case expression not constant
error C2051: case expression not constant

In my header file I have:

protected:
    char up_key;
    char down_key;
    char right_key;
    char left_key;

I am using Visual C++ 2008.

switch case expressions must be compile time constants. change those to `static const char up_key = 1;` and such, and problem solved. — Mooing Duck, Jan 19 '12 at 04:06
Because the Standard says so. It's a remnant of the old days, where `switch` were introduced as a "nicer" presentation that was transformed into an array look-up automagically (and thus required constants). Nowadays it makes less sense, but the syntax has not been changed so... — Matthieu M., Jan 19 '12 at 07:49

score 17 · Accepted Answer · answered Jan 19 '12 at 04:07

17

As the error message states, the case expressions must be constant. The compiler builds this as a very fast look-up table at compile time and it can't do that if there is a possibility that the values could change as the program runs.

If you do need them to be variable, not constant, your best bet is to use if/else statements instead.

answered Jan 19 '12 at 04:07

Janne

979
4
13

1

is it possible to somehow "see" the output of compiler in this case, and maybe compare it with if/else? – Wakan Tanka Sep 29 '16 at 09:15

score 7 · Answer 2 · edited Apr 26 '15 at 09:46

7

Replace this long clumsy code,

switch(c)
{
    case up_key:
        move(0, -1);
        break;

    case down_key:
        move(0, 1);
        break;

    case left_key:
        move(-1, 0);
        break;

    case right_key:
        move(1,0);
        break;

    default:
        return false;
}

with something like this:

move( (c==right_key) - (c==left_key) , (c==down_key) - (c==up_key) );

You can litterly replace that 17 lines long of code with that much more neat single line of code.

edited Apr 26 '15 at 09:46

Peter Mortensen

30,738
21
105
131

answered Dec 08 '13 at 19:59

user3080666

87
1
1

14

What makes you think that that code is neat? – EralpB Oct 19 '14 at 13:41
1

It *is* a neat line - maybe you should just add a small explanatory comment to the code to assist the hard-of-reading types ;) – slashmais Apr 19 '15 at 15:30
2

doesn't it increase time complexity? 4 comparisons, 4 type casts and 2 arthmatic ops are to be made with ur single line! – Necktwi Sep 16 '15 at 11:06
@neckTwi I don't think those extra operations matter, considering that a keypress doesn't happen very often. (Then it is also possible that the compiler might optimize that line to something that is equivalent to the switch–case statement above, although I don't know how much you can trust that it actually does) – HelloGoodbye Dec 02 '15 at 12:10
@EralpB It is neat because it consists of less code which _in my opinion_ 1) makes reading the code go faster, and 2) makes writing and maintaining the code less error-prone since you need to make changes to fewer places. – HelloGoodbye Dec 02 '15 at 12:16

score 1 · Answer 3 · answered Jan 19 '12 at 04:06

1

You can't because the language doesn't work that way. For example, what would happen if up_key, down_key, right_key, and left_key were all equal?

answered Jan 19 '12 at 04:06

MSN

53,214
7
75
105

,i see what you mean. But if i use if/else and they are all equal?? – infinitloop Jan 19 '12 at 04:08
Actually, you can have the same value here more than once, though you'll always end up in the first one in the list. With if/else, too, you'll get to the first one and never to the later ones. – Janne Jan 19 '12 at 04:12
3

Even if they were all constants, which is the crucial aspect here, the values in the cases must all be distinct. But the distinctness is not the problem - it is the lack of constness. – Jonathan Leffler Jan 19 '12 at 04:13
You can't have the same constant twice? I believe GCC allows it, though now you make me unsure. – Janne Jan 19 '12 at 04:14

score 1 · Answer 4 · answered Jan 19 '12 at 04:08

Because the switch statement can take only constants, you know when reading the code that the things you're comparing against are all constants. On the other hand, you would use if statements (or some other structure) to compare against variables:

if (c == up_key) {
    move(0, -1);
} else if (c == down_key) {
    move(0, 1);
} else ...

This provides a distinct difference in structure which can greatly aid those who come after you in reading your code. Imagine if you had to look up every case label to see whether it was a variable or not?

yup, that's what i am gonna do, thanks. – infinitloop Jan 19 '12 at 04:10 — infinitloop, Jan 19 '12 at 04:10

score 1 · Answer 5 · answered Jan 19 '12 at 04:25

1

I believe it's because the compiler generates a jump table, with the values hardcoded in, although I may be wrong. The way the tables are generated just doesn't allow for it.

answered Jan 19 '12 at 04:25

dymk

887
2
10
21

score 1 · Answer 6 · answered Jan 19 '12 at 04:40

Since other answers have covered why you are getting an error, here is a way to move in one of the four directions in response to a key press: use lookup tables instead of the conditionals/switches.

Setup portion:

std::map<char,pair<int,int> > moves;
moves[up_key] = make_pair(0, -1);
moves[down_key] = make_pair(0, 1);
moves[left_key] = make_pair(-1, 0);
moves[right_key] = make_pair(1, 0);

The function:

bool Character::keyPress(char c) {
    if (moves.count(c)) {
        pair<int,int> dir = moves[c];
        move(dir.first, dir.second);
        return true;
    } else {
        return false;
    }
}

score 0 · Answer 7 · answered Sep 11 '13 at 16:37

//here is the full functional code snippet which can be compiled and run with most of C++  
//compiler/link ...console app was demoed but you can apply the code/logic to win32 app...
//if you have any problem, send me email to Samuel_Ni@yahoo.com

#include <iostream.h>
#include <map>
#include <conio.h>

class CkbdHanler{
  private:
    map<char,pair<int,int> > moves;
  protected:
    char up_key;
    char down_key;
    char right_key;
    char left_key;
  public:

CkbdHanler(char a,char b,char c,char d):up_key(a),
                                        down_key(b),
                                       right_key(c),
                                       left_key(d)
{
    moves[up_key] = make_pair(0, -1);
    moves[down_key] = make_pair(0, 1);
    moves[left_key] = make_pair(-1, 0);
    moves[right_key] = make_pair(1, 0);
 }

bool keyPress(char c){
    if (moves.count(c)) {
            pair<int,int> dir = moves[c];
            move(dir.first, dir.second);
            return true;
   } else return false;

}
void move(int i,int j){
   cout<<"(i,j)=("<<i<<","<<j<<")"<<endl;
  }
};

int main(int argc, char* argv[])
{
  CkbdHanler CmyKbdH('u','d','l','r');

  cout << "Hello C++... here is a demo of Map to replace switch-case" << endl;
  CmyKbdH.keyPress('d');
  cout << endl << "Press any key to continue...";

  getch();
  return 0;
}

Why can't I have a variable in switch-case statement?

7 Answers7

Linked