8
votes

I'm writing some code with a simple switch statement based on Enum values. It occurred to me that at some point in future a developer may add a new value, so I included a default method to capture this at runtime and throw an exception. However I realised that I should do this every time I put in logic like this, and that I'd only see such issues at run time rather than compile time.
I'm wonderring if there's some code I can add to get the compiler to tell the developer that they need to update certain methods in the case that they update enum values - beyond just adding comments to the enum itself?

e.g. (the example below's purely theoretical; I chose statuses from the development lifecycle to ensure it's something familiar to most).

public enum DevelopmentStatusEnum
{
    Development
    //, QA //this may be added at some point in the future (or any other status could be)
    , SIT
    , UAT
    , Production
}

    public class Example
    {
        public void ExampleMethod(DevelopmentStatusEnum status)
        {
            switch (status)
            {
                case DevelopmentStatusEnum.Development: DoSomething(); break;
                case DevelopmentStatusEnum.SIT: DoSomething(); break;
                case DevelopmentStatusEnum.UAT: DoSomething(); break;
                case DevelopmentStatusEnum.Production: DoSomething(); break;
                default: throw new StupidProgrammerException(); //I'd like the compiler to ensure that this line never runs, even if a programmer edits the values available to the enum, alerting the program to add a new case statement for the new enum value
            }
        }
        public void DoSomething() { }
    }
    public class StupidProgrammerException: InvalidOperationException { }

This is a bit academic, but I can see it as being useful in making my app robust. Has anyone tried this before / got any good ideas on how this might be achieved?

Thanks in advance,

JB

4
I do it the same way that you do.David Heffernan
Everytime you find yourself writing a switch statement you should consider refactoring the code into using polymorphism instead. See this exerpt of Martin Fowler's book Refactoring. This will make your code adhere to the Open-Closed Principle (OCP) and help the next developer avoid missing to change a switch statement somewhere. :-)Ulf Åkerstedt

4 Answers

13
votes

In such a case I would try not to use an enum but rather a class with public static readonly fields which are instances of the class. See what the .Net framework does with colors for example. There is a class Color and you can use objects like Color.Black, Color.Blue, etc. They're not constants but offer almost all the same benefits. Plus they have other benefits that constants don't have. See the C# version 3 language specification which also talks about this a bit.

But the idea is that you don't have a case statement. You add enough other properties to each "enum" member that the method (DoSomething or whatever) can treat it properly. When another developer wants to add another member object, he needs to supply the attributes needed. My example: I needed an "enum" for the different actions a user can take in the system. These actions needed to be checked for permissions, logged, etc. I also needed parent and child actions (renaming something is "part of" editing it, etc.), abstract actions used for grouping actions together for filtering purposes, and special actions "All" and "None" (None being undefined). Each one needed an ID and text in the database as well. I wanted it to still work if someody invented a new type of action. What I did is something like this (lots of code omitted just to give you the idea):

  public class Action
  {
    protected Action(bool Abstract, Action Parent, int ID, string Name, bool Undefined)
    { /* snip */ }
    protected Action(bool Abstract, Action Parent, int ID, string Name)
      : this(Abstract, Parent, ID, Name, false)
    { }
    //----------------------------------------------------------------------------------------
    public static readonly Action All = new Action(true, null, 0, "All");
    public static readonly Action None = new Action(false, All, 6, "(Undefined)", true);
    public static readonly Action Modifying = new Action(true, All, 1, "Modifying");
    public static readonly Action Creating = new Action(false, Modifying, 2, "Creating");
    public static readonly Action Deleting = new Action(false, Modifying, 3, "Deleting");
    public static readonly Action Editing = new Action(false, Modifying, 4, "Editing");
    public static readonly Action Exporting = new Action(false, All, 5, "Exporting");
    public static readonly Action Renaming = new Action(false, Editing, 7, "Renaming");
    /* snip */
    //----------------------------------------------------------------------------------------
    /* template for new entries:
    public static readonly Action  = new Action(false, All, , "");
    */
  }

There are more actions. And there are a number of methods in other classes that operate on actions. They all keep working as long as each action provides the information needed. The developer adding an action is forced to provide the information. If the current attributes are not enough for some future "special" action then more attributes will have to be added later. Note that the constructors are protected, so only the class itself can create actions. I omitted a lot of code in the main constructor that checks for duplicate IDs and Names and lots of other things. You can now use it like this:

Log.LogAction(Action.Renaming);

The method LogAction doesn't have a case statement in it. It uses the attributes of the action.

What is your enumeration?

Regards

3
votes

I've found a 'solution' using plain Enums. Here it is, raw, maybe it could be improved with a better design:

bool allCasesHandled;

switch (myEnumValue)
{
    case MyEnum.Value1:
        allCasesHandled = true;
        break;

    //default:
    //  allCasesHandled = true;
    //  break;
}
System.Diagnostics.Debug.WriteLine(allCasesHandled);

If you try to compile this, you will get an error 'use of an unassigned variable'.

This is a bit cumbersome to maintain, but for simple cases, it might be useful, especially with a comment on the line that is meant to fail.

2
votes

I might be wrong, but I don't think, the compiler offers such warnings. You can can catch such issues with some unit tests, that invoke a method like the one above with all possible enum values (use Enum.GetValues() for this). Every time, a developer adds an enum member and forgets to modify all the switch statements, at least one unit test will fail with a "StupidProgrammerException" (btw: I would throw an ArgumentOutOfRangeException).

0
votes

I think you can write rules for StyleCop and run them in a PostBuild event, and they can output warnings in the Build window. We started attempting this awhile back to add warning for ignoring the return value of a method, but never got around to finishing it. Last I looked into it, you needed to analyze the IL, which isn't always fun. Of course, I guess that depends on your definition of fun.