Switch Statement – Handling Unreachable Default Case

enummaintenanceunit testing

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?

Best Answer

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.

Related Solutions

Enum – How to Use a Switch Statement with Enum Efficiently

Your scenario doesn't sound like an appropriate use for an enumeration. Also, unless the values never change, this should be data driven.

If the values never change, then a key-value pair is more appropriate where the destination is the key and the flight number is the value. This would eliminate any need for a switch statement, as the key can be used to directly and efficiently locate the value.

if you really want to use an enum, then the switch statement in C# would be something like:

int flightNumber;
Destination selection = // some value collected from user input;

switch( selection )
{
    case Destination.London:
        flightNumber = 201;
    break;

    // etc.
}

Design Practices – Is Switch-Case Over Enumeration Bad Practice?

A "nicer" way is to have three methods, one per command.

There is no need to collapse all those commands into one method and to use a switch later. Since those commands do different things, they deserve their own methods in the interface.

Instead of:

public void ChangeState(PlayerState newState)
{
    switch (newState)
    {
        case PlayerState.Play:
            // Start playing.

        case PlayerState.Stop:
            // Stop playing.

        case PlayerState.Pause:
            // Pause or resume.
    }
}

you would have:

public void Play()
{
    // Start playing.
}

public void Stop()
{
    // Stop playing.
}

public void Pause()
{
    // Pause or resume.
}

Why?

Your current implementation using a switch will:

either do multiple things,
or will just serve to call Play, Stop and Pause methods.

In the first case, you break the rule which says that a method should do one and one only thing.

In the second case, KISS: don't write a method you really don't need and which doesn't bring anything useful to the API.