5

I'm using Java 6.

Suppose I have an enum with 6 values, ordered A to F. About 4 of the values were handled the same. I could write it like this.

switch (whichType) {
    case A:
    case B:
    case C:
    case D:
        return task();
    case E:
        return someothertask();
    case F:
        return anothersomeothertask();
}

Or like this.

switch (whichType) {
    case E:
        return someothertask();
    case F:
        return anothersomeothertask();
    default:
        return task();
}

Null values will never reach this switch.

In terms of conciseness and clarity, the second approach is better. In terms of being explicit, I think the first approach is better.

Are there any other pros/cons of each approach?

Also, this simple question risks being a duplicate, but I tried and couldn't find it asked anywhere yet. I apologize if I haven't searched it good enough.

Russell
  • 3,975
  • 7
  • 37
  • 47
  • 4
    add an abstract method to your enum class? – Ron Jan 11 '11 at 03:33
  • 2
    Just for the record: you _can_ put a `default` section at the top, in the middle, or anywhere else within the `switch`; it doesn't have to be at the bottom. – C. K. Young Jan 11 '11 at 03:38
  • @ChrisJester-Young But some static code analyzers prohibit putting default in non-bottom positions. – mjafari Sep 22 '12 at 08:23
  • @mjafari: It's not my fault if they have arbitrary requirements like that. :-P I understand that some people (such as people who write such restrictions into their tools) think non-bottom `default` clauses are bad style, but I disagree and I would not hesitate to use it if it makes the code clearer. – C. K. Young Sep 22 '12 at 20:26

5 Answers5

7

Both are fine if that enum is absolutely, positively fixed at six values forever. Otherwise, think about what a possible seventh value for the enum might be. If E and F are the only two possible outliers with respect to this switch logic and any other value would fall in the same bucket as A through D, go ahead and use the default. Otherwise, you're safer to have a case per value.

dkarp
  • 14,483
  • 6
  • 58
  • 65
3

Suppose someone adds a value to the enumeration?

In some code I've got, there's this sort of construct:

switch(mediaType) {
    case typeA:
    case typeB:
    case typeC:
        DoGeneric();
        break;
    case TypeD:
        DoSpecial();
        break;
}

And the reason for that is so that if someone adds another possibility to the enumeration, it won't compile until you've looked at the places the enumeration is used and decided what needs to be done at that point. No chance of missing one and having a difficult-to-track-down bug.

Anon.
  • 58,739
  • 8
  • 81
  • 86
  • 5
    Ummm ... that code **will** compile after someone adds `typeE`. – Stephen C Jan 11 '11 at 03:34
  • @Stephen C: Depending on language, yes it might (I don't develop in Java). But this way, your code style tools can pick up on it, while with `default` it's impossible to tell whether it's been forgotten or not. – Anon. Jan 11 '11 at 03:38
2

I tend to use default to catch new enums added after the initial implementation. This doesn't produce a compile-time error, which I'd prefer, but it generally doesn't get past smoke-testing.

switch (whichType) {
    case A:
    case B:
    case C:
    case D:
        return task();
    case E:
        return someothertask();
    case F:
        return anothersomeothertask();
    default:
        throw new IllegalArgumentException("Encountered unknown type: " + whichType.name());
}
Adam Mlodzinski
  • 3,728
  • 2
  • 13
  • 10
2

Well, with respect to readability, I would argue that the second may not be more clear. For example, your common case happens AFTER your special cases.

However, the real problem I see here is the two pieces of code aren't really equivalent. In the first case, you have six explicit cases. In the second, you have two explicit cases, and one default case. This means that, if your code has some sort of error where you receive a case you didn't expect, the default case will still perform in the second (and task() will still run). As a result, this is fundamentally different from your first code.

Other than that (deciding which switch statement is correct), I would say stick with the one that reads the best to you. I personally like the first (because it is explicit and leaves room for a default case if one is needed), but you'll have to decide based on your preferences or your company's/team's development standards.

JasCav
  • 34,458
  • 20
  • 113
  • 170
0

If you are writing some business scenario, and you expect the code to live and to be maintained (by some other person) for years to come, go with option 1. There is nothing wrong with option 2, it is short and clear, but when talking about maintainability, I think option 1 wins - especially if the cases are related to some business logic. It will be far easier for the maintaining engineer to read it.

ring bearer
  • 20,383
  • 7
  • 59
  • 72