I just stumbled across this guideline in the Design Guidelines document and I thought it needed more explanation..

 

Do argument validation. Do not assume enum arguments will be in the defined range. It is legal to cast any integer value into an enum even if the value is not defined in the enum.

public void PickColor(Color color) {

   switch (color) {

          case Red:

                 ...

                 break;

          case Blue:

                 ...

                 break;

          case Green:

                 ...

                 break;

          //repeat for all known values of Color

          default:

                 throw new ArgumentOutOfRangeException();

                 break;

    }

    //do work
}

 

Note: do not use Enum.IsDefined() for these checks as it is based on the runtime type of the enum which can change out of sync with this code. 

 

There are really two problems with Enum.IsDefined().  First it loads reflection and a bunch of cold type metadata making it a deceptively expensive call. 

Secondly, as the note alludes to there is a versioning issue here.  Consider an alternate way to write the method defined above:

public void PickColor(Color color) {

   if (!Enum.IsDefined (typeof(Color), color) {
       throw new InvalidEnumArgumentException("color", (int) color, typeof(Color));

   }

   //Security issue: never pass a negative color value

   NativeMethods.SetImageColor (color, byte[] image);

}

 

Callsite:

Foo.PickColor ((Color) -1); //throws InvalidEnumArgumentException()

 

This looks pretty good, even if you know this (mythical) native API has a buffer overrun if you pass a color value that is negative.   You know this because you know the enum only defined postive values and you are sure that any value passed was one of the ones defined in the enum right?  Well, only ½ right.  You don’t know what values are defined in the enum.  Checking at the moment you write this code is not good enough because IsDefined takes the value of the enum at runtime.  So if later someone added a new value (say Ultraviolate = -1) to the enum IsDefined will start allowing the value -1 one through.  This is true whether the enum is defined in the same assembly as the method or another assembly. 

 

public Color {
   Red = 1,
   Green = 2,
   Blue = 3,
   Ultraviolate = -1, //new value added this version
}

Now, that same callsite no longer throws.

Callsite:

Foo.PickColor ((Color) -1); //causes a buffer overrun in NativeMethods.SetImageColor()

 

The moral of the story is also two fold.  First be very careful when you use Enum.IsDefined in your code.  The Second is when you design an API to simplify a situation, but sure the fix isn’t worse than the current problem.