IntelliJ complaining "for statement does not loop"?

Question

This is the code I have:

public enum Modification {
    NONE, SET, REMOVE;
}

boolean foo(){
    for (S s : sList) {
        final Modification modification = s.getModification();
        switch (modification) {
            case SET:
            case REMOVE:
                return true;
            /*
            case NONE:
                break;
            */
        }
    }
    return false;
}

And when the code is as seen above, IntelliJ will say:

'for' statement does not loop less... () Reports any instance of for, while and do statements whose bodies are guaranteed to execute at most once. Normally, this is an indication of a bug.

Only if I make the following change, IntelliJ will be happy:

for (S s : sList) {
    final Modification modification = s.getModification();
    switch (modification) {
        case SET:
        case REMOVE:
            return true;
        case NONE:
            break;
    }
}

Why is my for loop not looping if case NONE: is not included in the switch statement?

Answer 1

I just tried this in eclipse and you end up with a compiler warning on the switch statement.

The enum constant NONE needs a corresponding case label in this enum switch on Modification

To resolve the warning I'm given the following options.

• Add default case
• Add missing case statements
• Add @SuppressWarnings 'incomplete-switch' to foo()

If I add the missing case statement then the warning no longer appears. The same as adding the missing case makes your error warning disappear from intellij.

Without the statement for case NONE you can only see two cases, both of which return true. Without knowing the structure of Modification and the extra value of NONE it looks like this loop would just return true on the first iteration of the loop.

Of course the compiler should actually know that there are more values for Modification than SET and REMOVE so the warning is just for good style. Basically your code works but here's how to improve it.

I would choose to add a default statement rather than the missing case. This would be more future proof in case more values are later added to the enum. E.G.

switch (modification) 
{
  case SET:
  case REMOVE:
    return true;
  default:
    break;
}

Personally I'm not a fan of using the fall through on switch statements. What you gain in making the code concise you lose in legibility IMHO. If someone later comes and adds a case between SET and REMOVE it could introduce a bug. Also, having a return statement mid-way through a method can also cause problems. If someone wants to add some code just before the return they may miss all the places. If the method is very simple then multiple returns is fine but you've stated that this is a simplified example and so if this block of code is complicated I would avoid it.

If you're able to use Java 8 then this looks to be the perfect use case for the new stream API. Something like the following should work.

return sList.stream().anyMatch(
  modification -> (modification==Modification.SET || modification==Modification.REMOVE)
);