20

If I'm using a switch statement to handle values from an enum (which is owned by my class) and I have a case for each possible value - is it worth adding code to handle the "default" case?

enum MyEnum
{
    MyFoo,
    MyBar,
    MyBat
}

MyEnum myEnum = GetMyEnum();
switch (myEnum)
{
    case MyFoo:
        DoFoo();
        break;
    case MyBar:
        DoBar();
        break;
    case MyBat:
        DoBat();
        break;
    default:
        Log("Unexpected value");
        throw new ArgumentException() 
}

I don't think it is because this code can never be reached (even with unit tests). My co-worker disagrees and thinks this protects us against unexpected behavior caused by new values being added to MyEnum.

What say you, community?

s d
  • 322
  • 1
  • 2
  • 9
  • Let's say MyEnum is a non-nullable type. – s d May 03 '12 at 17:43
  • 3
    "Today" it's non-nullable. What about tomorrow when you're not maintaining the code anymore. Or what about when "MyBiz" is added to the enum but not the case? Caleb's comments on maintenance are very germane. –  May 03 '12 at 18:34
  • 1
    Teach your compiler that it is a fatal error if there is a switch not covering all cases. –  May 03 '12 at 21:29
  • What if someone casts an invalid value to `MyEnum` then passes it through your switch? – Mawg says reinstate Monica Feb 09 '18 at 09:49
  • 1
    What language? If Java, you should put a method inside the Enum and just call it (polymorphism), eliminating the `switch` statement entirely. – user949300 Nov 13 '18 at 07:25
  • I'd use `default: assert(0 && "unexpected enum value");` in such a case. Make it fail hard, immediately, and trivially debugable instead of just throwing an exception that might be caught by some indirect caller. – cmaster - reinstate monica Nov 13 '18 at 11:42
  • When I wrote this question back in the early 70s, the language was C# - but I'm interested in solutions in any language (that supports switch statements). – s d Nov 13 '18 at 19:02

5 Answers5

37

Including the default case doesn't change the way your code works, but it does make your code more maintainable. By making the code break in an obvious way (log a message and throw an exception), you're including a big red arrow for the intern that your company hires next summer to add a couple features. The arrow says: "Hey, you! Yes, I'm talking to YOU! If you're going to add another value to the enum, you'd better add a case here too." That extra effort might add a few bytes to the compiled program, which is something to consider. But it'll also save someone (maybe even the future you) somewhere between an hour and a day of unproductive head-scratching.

Update: The situation described above, i.e. protecting against values added to an enumeration at some later time, can also be caught by the compiler. Clang (and gcc, I think) will by default issue a warning if you switch on an enumerated type but don't have a case that covers every possible value in the enumeration. So, for example, if you remove the default case from your switch and add a new value MyBaz to the enumeration, you'll get a warning that says:

Enumeration value 'MyBaz' not handled in switch

Letting the compiler detect uncovered cases is that it largely eliminates the need for that unreachable default case that inspired your question in the first place.

Caleb
  • 38,959
  • 8
  • 94
  • 152
  • 2
    Ok, you’ve convinced me :) I’ll just have to accept the dent in my code coverage numbers. – s d May 04 '12 at 14:40
  • @st There's no reason that you can't test that code. Just do a test build that conditionally compiles in an extra value in your enumeration, and then write a unit test that uses it. Perhaps that's not ideal, but it's probably not the only case where you need to test code that normally will never be reached. – Caleb May 04 '12 at 14:52
  • Or, you cast a non-enum value to your enum type and use that. – Mawg says reinstate Monica Feb 09 '18 at 09:50
7

I was just talking with a co-worker about this this morning as well -- it's really unfortunate, but I think handling the default is required for safety, for two reasons:

First, as your co-worker mentions, it future-proofs the code against new values being added to the enum. This may or may not seem like a possibility, but it's always there.

More importantly, depending on the language/compiler, it may be possible to have values that aren't members of the enum in your switched variable. For example, in C#:

MyEnum myEnum = (MyEnum) 3; // This could come from anywhere, maybe parsed from text?
// ... code goes on for a while

switch ( myEnum )
{
    case MyEnum.A:
        // ... handle A case
        break;
    case MyEnum.B:
        // ... handle B case
        break;
}

// ... code that expects either A or B to have happened

By adding the simple case default: and throwing an exception, you've protected yourself against this weird case where "nothing" happens but "something" should have happened.

Unfortunately, basically any time I write a switch statement anymore, it's because I'm checking the cases of an enum. I really wish the "throw by default" behavior could be enforced by the language itself (at least by adding a keyword).

Chris Phillips
  • 253
  • 1
  • 8
3

Adding a Default case even if you never expect to reach it can be a good thing. It will make debugging much easier if your code throws a "This shouldn't have happened" exception right away rather than later in the program, trowing some mysterious exception or returning unexpected results with no error.

Ryathal
  • 13,317
  • 1
  • 33
  • 48
2

I say:

Try adding another type to MyEnum. Then change this line:

MyEnum myEnum = GetMyEnum();

to 

MyEnum myEnum = SomethingElse;

Then run your code with the default case and without the default case. Which behaviour do you prefer?

Having the default case can also be useful for trapping NULL values and preventing NullPointerExceptions.

FrustratedWithFormsDesigner
  • 46,105
  • 7
  • 126
  • 176
-1

If you had any idea how mach time can be saved by insisting on the default case, you would not need to ask the question. Silently doing nothing in an error condition is not acceptable- do you silently catch exceptions that should never happen? Leaving "mines" for programmers that follow you, similarly unacceptable.

If your code is never going to be changed or modified, and is 100% bug free, leaving out the default case may be OK.

Ada (the grandfather of robust programming languages) won't compile a switch on an enum, unless all the enums are covered or there is a default handler - this feature is on my wish list for every language. Our Ada coding standard states that with switches on enums, explicit handling of all values, with no default is the preferred way to handle this situation.

mattnz
  • 21,315
  • 5
  • 54
  • 83