A Switch Java problem : case expressions must be constant expressions

Question

I having a problem in my switch/case statement. The error says : "Case expressions must be constant expressions". I understand the error and I can resolve it using If but can someone tells me why the case expression must be constant in a switch/case. A code example of my error :

public boolean onOptionsItemSelected(MenuItem item) {
    int idDirectory = ((MenuItem) findViewById(R.id.createDirectory)).getItemId();
    int idSuppression = ((MenuItem) findViewById(R.id.recycleTrash)).getItemId();
    int idSeeTrash = ((MenuItem) findViewById(R.id.seeTrash)).getItemId();

    switch (item.getItemId()) {
    case idDirectory:
        createDirectory(currentDirectory);
        break;
    case idSuppression:
        recycleTrash();
        break;
    case idSeeTrash:
        seeTrash();
        break;
    }

    return super.onOptionsItemSelected(item);
}

Thx for your explanation !!

Solution 1

So it can be evaluated during the compilation phase ( statically check )

See: http://docs.oracle.com/javase/specs/jls/se7/html/jls-14.html#jls-14.11 for a formal definition of the switch.

Additionally it may help you to understand better how that switch is transformed into bytecode:

class Switch {
  void x(int n ) {
    switch( n ) {
      case 1: System.out.println("one"); break;
      case 9: System.out.println("nine"); break;
      default:  System.out.println("nothing"); break;
    }
  }
}

And after compiling:

C:\>javap -c Switch
Compiled from "Switch.java"
class Switch extends java.lang.Object{
Switch();
  Code:
   0:   aload_0
   1:   invokespecial   #1; //Method java/lang/Object."<init>":()V
   4:   return

void x(int);
  Code:
   0:   iload_1
   1:   lookupswitch{ //2
                1: 28;
                9: 39;
                default: 50 }
   28:  getstatic       #2; //Field java/lang/System.out:Ljava/io/PrintStream;
   31:  ldc     #3; //String one
   33:  invokevirtual   #4; //Method java/io/PrintStream.println:(Ljava/lang/String;)V
   36:  goto    58
   39:  getstatic       #2; //Field java/lang/System.out:Ljava/io/PrintStream;
   42:  ldc     #5; //String nine
   44:  invokevirtual   #4; //Method java/io/PrintStream.println:(Ljava/lang/String;)V
   47:  goto    58
   50:  getstatic       #2; //Field java/lang/System.out:Ljava/io/PrintStream;
   53:  ldc     #6; //String nothing
   55:  invokevirtual   #4; //Method java/io/PrintStream.println:(Ljava/lang/String;)V
   58:  return

}

See that line marked as 1:

 1:   lookupswitch{ //2
            1: 28;
            9: 39;
            default: 50 }

It evaluates the value and goes to some other line. For instance if value is 9 it will jump to instruction 39:

   39:  getstatic       #2; //Field java/lang/System.out:Ljava/io/PrintStream;
   42:  ldc     #5; //String nine
   44:  invokevirtual   #4; //Method java/io/PrintStream.println:(Ljava/lang/String;)V
   47:  goto    58

Which in turn jumps to instruction 58 :

   58:  return

All this wouldn't be possible if it was evaluated dynamically. That's why.

Solution 2

in eclipse IDE is simple ,in switch sentence CTRL + 1 and convert switch sentence - if-else sentence http://tools.android.com/tips/non-constant-fields

Solution 3

The idDirectory and others need to be a constant and not a declared variable. Switch will not work in this case, you need switch to if-else construct.

EDIT I see what OP meant. That is just how switch works in java language.

Comments

[Anon.]

Why are you using findViewById().getItemId()? If you already have the id, you don't need to get the item in order to get the id!

[Bnjmn]

switch statements are a bad smell... I recommend you look into using polymorphism or the adapter pattern in order to achieve better looking code. Additionally you can avoid problems like these altogether.

[OscarRyz]

@Bnjmn I agree, specially when the condition repeats quite often. If you use it seldom it may be ok. Also, I use it sometimes with polymorphism to create the initial concrete instance.

[idbrii]

@Bnjmn: Since Dimitri's using Android, his MenuItem objects are likely defined in xml and (to my knowledge) cannot be easily subclassed without abandoning the simplicity of MenuInflater.inflate().

[Will Tate]

This is exactly your problem. If you can't get around this with constants just use a slightly more complex if-then-else-if-blah-blah setup.

[OscarRyz]

-1 For answering what the OP already knows: I understand the error and I can resolve it using If but can someone tells me why the case expression must be constant in a switch/case

[Dimitri]

Thanks man, that's a nice answer. So if compile with javap, i can see the bytecode generated??

[OscarRyz]

@Dimitri. You're welcome. javap is another tool ( like javadoc or jar ) which is the default java dissambler. If invoked with -c you'll see these instructions.

[Dimitri]

Ok. But i was thinking, do you think the bytecode generated in the JVM is the same in Dalvik Machine (Android). I am working on Android as you may see it

[OscarRyz]

Actually... it is not the same. Dalvik has its own codes, but the switch will work the same in both. Take a good look at: en.wikipedia.org/wiki/Dalvik_(software)