Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
    
Let's say MyEnum is a non-nullable type. –  s d May 3 '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. –  GlenH7 May 3 '12 at 18:34
1  
Teach your compiler that it is a fatal error if there is a switch not covering all cases. –  user1249 May 3 '12 at 21:29
add comment

5 Answers

up vote 27 down vote accepted

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.

share|improve this answer
1  
Ok, you’ve convinced me :) I’ll just have to accept the dent in my code coverage numbers. –  s d May 4 '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 4 '12 at 14:52
add comment

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).

share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
add comment

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.

share|improve this answer
    
Why all the -1's? –  mattnz May 4 '12 at 7:44
1  
I didn't -1 you, but I figure it'd be because of your attitude ;) –  Friek May 4 '12 at 10:34
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.