Programming against an enum in a switch statement, is this your way to do?

Question

Look at the code snippet:

This is what I normally do when coding against an enum. I have a default escape with an InvalidOperationException (I do not use ArgumentException or one of its derivals because the coding is against a private instance field an not an incoming parameter).

I was wondering if you fellow developers are coding also with this escape in mind....

public enum DrivingState {Neutral, Drive, Parking, Reverse};

public class MyHelper
{
    private DrivingState drivingState = DrivingState.Neutral;

    public void Run()
    {
        switch (this.drivingState)
        {
            case DrivingState.Neutral:
                DoNeutral();
                break;
            case DrivingState.Drive:
                DoDrive();
                break;
            case DrivingState.Parking:
                DoPark();
                break;
            case DrivingState.Reverse:
                DoReverse();
                break;
            default:
                throw new InvalidOperationException(
                    string.Format(CultureInfo.CurrentCulture, 
                    "Drivestate {0} is an unknown state", this.drivingState));
        }
    }
}

In code reviews I encounter many implementations with only a break statement in the default escape. It could be an issue over time....

Answer 1

Your question was kinda vague, but as I understand it, you are asking us if your coding style is good. I usually judge coding style by how readable it is.

I read the code once and I understood it. So, in my humble opinion, your code is an example of good coding style.

Answer 2

There's an alternative to this, which is to use something similar to Java's enums. Private nested types allow for a "stricter" enum where the only "invalid" value available at compile-time is null. Here's an example:

using System;

public abstract class DrivingState
{
    public static readonly DrivingState Neutral = new NeutralState();
    public static readonly DrivingState Drive = new DriveState();
    public static readonly DrivingState Parking = new ParkingState();
    public static readonly DrivingState Reverse = new ReverseState();

    // Only nested classes can derive from this
    private DrivingState() {}

    public abstract void Go();

    private class NeutralState : DrivingState
    {
        public override void Go()
        {
            Console.WriteLine("Not going anywhere...");
        }
    }

    private class DriveState : DrivingState
    {
        public override void Go()
        {
            Console.WriteLine("Cruising...");
        }
    }

    private class ParkingState : DrivingState
    {
        public override void Go()
        {
            Console.WriteLine("Can't drive with the handbrake on...");
        }
    }

    private class ReverseState : DrivingState
    {
        public override void Go()
        {
            Console.WriteLine("Watch out behind me!");
        }
    }
}

Answer 3

I don't like this approach because the default case is untestable. This leads to reduced coverage in your unit tests, which while isn't necessarily the end of the world, annoys obsessive-compulsive me.

I would prefer to simply unit test each case and have an additional assertion that there are only four possible cases. If anyone ever added new enum values, a unit test would break.

Something like

[Test]
public void ShouldOnlyHaveFourStates()
{
    Assert.That(Enum.GetValues( typeof( DrivingState) ).Length == 4, "Update unit tests for your new DrivingState!!!");
}