Any better way to write this “if” statement?

Question

if( (scene==1) || (scene==2) || (scene==3) || (scene==4) || .... (scene==n) ) {
    ....
    ....
}

How can we optimize the condition in the if statement? Any better logic to get the same result?

Any help in this regard will be well appreciated. If this question is below standard, then extremely sorry for that.

Answer 1

With these values you could try something like this :

if(scene >= 1 && scene <= n) {
    ...
}

If your values are not contiguous, see solutions suggested in Stephen C's or Donald.McLean's post.

Answer 2

Another alternative is to use a switch statement:

switch (scene) {
case 1:
case 2:
case 3:
case 42:
     // do something
     break;
default:
     // do something else
}

Which approach is best (i.e. most readable) will depend on the set of values you need to test against. I wouldn't worry about efficiency at this level, unless your profiling tells you that this is a significant hotspot.

Answer 3

For ranges of contiguous ranges of values, Autar's answer is your best bet.

For non-contiguous ranges, or for more complex values (Strings), I would suggest a Set.

private Set<Integer> criteriaValues;

// then in a constructor or initialization block
criteriaValues = new HashSet<Integer>();
// add some values to the set here.

The comparison becomes:

if (criteriaValues.contains(scene)) {
    ...
}

Answer 4

Assuming scene isn't an int,

if ( scene == (int) scene && (scene >= 1 && scene <= n) ){
    ...
}

Answer 5

I generally do something like:

vals = [ 1 , 2 , 3 , 4 ]
if ( scene in vals) ...

Answer 6

I think, such long condition like this should be the method. In method you can use any way to determine that condition is true. I would prefer the searching in the array.

boolean sceneIsSomething(int scene) {
    int somethings = [ 1, 2, 3, 4 ];
    return somethings.contains(scene);
}

if (sceneIsSomething(scene)) {
    // do something you want ...
}

Answer 7

I strongly recommend Enums for that kind of statements. For example you can create an enum Scene where you map 1,2,3 ... to real instances as ONE, TWO, ... When implementing your if statement you implement a method in your enum. For example:

if(scene.isSceneRelevantForThis()){
  // do stuff
}