Design Guidelines Update: Enum Design

This is a recent update to the Design Guidelines. One of the most interesting additions is the section about adding values to enums (at the end of the section). This was one of the guidelines were getting an agreement across the whole company was quite challenging. Kit (frequent poster to the BCL Team Blog) spent days getting consensus for the guideline.

Enum Design

* Do use an Enum to strongly type parameters, properties, and return values.

Always define enumerated values using an enum if they are used in a parameter or property. This allows development tools to know the possible values for a property or parameter.

public enum TypeCode{

    Boolean,

    Byte,

    Char,

    DateTime,

    …

}

 

Convert.ChangeType(object value, TypeCode typeCode);

* Do favor using an enum over static constants.

* Do not use an enum for open sets (such as the operating system version etc).

* Do not extend System.Enum as a base class for a user-defined type.

System.Enum is used by the Common Language Runtime to create user-defined enumerations. Most programming languages provide a programming element that gives you access to this functionality. For example, in C# the enum keyword is used to define an enumeration.

* Do use the System.FlagsAttribute custom attribute for an enum only if a bitwise OR operation is to be performed on the numeric values[1]. Do use powers of two for the enum values so they can be easily combined.

[Flags()]
public enum WatcherChangeTypes {

    Created = 1,

    Deleted = 2,

    Changed = 4,

    Renamed = 8,

    All = Created | Deleted | Changed | Renamed

}

* Consider providing named constants for commonly used combinations of flags. Using the bitwise or is an advanced concept and should not be required for simple tasks.

[Flags()]

public enum FileAccess {

    Read = 1,

    Write = 2,

    ReadWrite = Read | Write,

}

            Avoid using flag enum values normal members that are negative or zero.

 

Negative values produce unexpected/confusing results in bitwise operations. An enum value of zero creates problems with and operations, etc:

[Flags]

public enum SomeFlag {

ValueA = 0, //bad design, don’t use 0 in flags

ValueB = 1,

ValueC = 2,

ValueBAndC = ValueB | ValueC,

}

 

SomeFlag flags = GetFlags();

if (flags & SomeFlag.ValueA) { \\ this will never evaluate to true

}

Annotation (AndersH): Note that in C# the literal constant 0 implicitly converts to any enum type, so you could just write:

if (Foo.SomeFlag == 0)…

We support this special conversion to provide programmers with a consistent way of writing the default value of an enum type (which, by CLR decree is "all bits zero" for any value type).

 

* Do name any value of zero on your flags enum ‘None’

For a flags enum, the zero-value is especially significant, and correlates to ‘no values set’

[Flags]

public enum BorderStyle

{

Fixed3D = 0x1,

FixedSingle = 0x2,

None = 0x0

}

if (foo.BorderStyle == BorderStyle.None)....

Annotation (Peter Golde): I have found the easiest way to test to see if a given flag is set in an enum is:
if ((value & flag) == flag)

 

Annotation (Vance Morrison): The rational for avoiding zero in a flag enumeration for an actual flag (only the special enum member ‘None’ should have zero) is that you can’t OR it in with other flags as expected.

However, notice that this rule only applies to Flag enumerations; in the case where enumeration is NOT a flag enumeration, there is a real disadvantage of avoiding zero that we have discovered however. ALL enumerations begin their life with this value (memory is zeroed by default). Thus if you avoid zero, every enumeration has an ILLEGAL value in it when it first starts its life in the run time (we can’t even pretty print it properly). This seems bad.

In my own coding, I do one of two things for non-flag enumerations.

If there is an obvious default that is highly unlikely to cause grief if programmers forget to set it (program invariants do NOT depend on it), I make this the zero case. Usually this is the most common value, which makes the code a bit more efficient (it is easier to set memory to 0 than to any other value).

If no such default exists, I make zero my ‘error’ (non-of-the-above) enumeration value. That way when people forget to set it, some assert will fire later and we will find problem.

In either case, however, from the compiler (and runtime), point of view every enumeration has a value for 0 (which means we can pretty print it).

 

* Do provide a value of zero on your non-flags enum

If None is not appropriate for the enum, then assign the zero-value to the element which should be used as the default value for the enum.

If the enum can logically support the concept of ‘None’, then add a None value to the enum, and give it the zero value

* Avoid creating flags enums where the design means that certain combinations of values are invalid

For example, The RegexOptions enum contains an ‘EcmaScript’ value, which cannot be combined with other elements in the enum (ExplicitCapture, RightToLeft, SingleLine, etc.), and if it is, an exception is thrown by APIs accepting the instance. It should have been supplied as a separate enum, or as a property.

It can be valid to combine elements in this way when simplicity is preferred, and there is relatively low risk of the concepts causing confusion (based on the clarity of the concept, and the names of the elements). For example, DateTimeStyles includes two values, ‘AssumeLocal’ and ‘AssumeUniversal’ that directly oppose each-other, but have been included in the existing enum to ensure fewer new APIs are necessary (which are potentially confusing), and existing code can more easily leverage the new concept.

If you have values in the enum which cannot be combined, either separate those values into a separate enum, or include them as properties on the class to which the enum is applied.

<Wrong way>

[Flags]

enum RegexOptions {

   Compiled = 0x01,

   SingleLine = 0x02,

   ECMAScript = 0x04,

   IgnoreCase = 0x08,

   None = 0x00

}

class SomeClass {

   public RegexOptions RegExOptions {

get { return _regexOptions; }

set {

if (value & (RegexOptions.ECMAScript | RegexOptions.SingleLine) == ((RegexOptions.ECMAScript | RegexOptions.SingleLine)) {

throw new ArgumentException (“invalid Argument”);

}

        }

}

}

<Right way>

[Flags]

enum RegexOptions {

   Compiled = 0x01,

   SingleLine = 0x02,

   ECMAScript = 0x04,

   IgnoreCase = 0x08,

   None = 0x00

}

enum RegexMode {

   Standard = 0x00,

   ECMAScript = 0x01

}

class SomeClass {

   public RegexOptions RegexOptions {

get { return _regexOptions; }

set {

if (((value & RegexOptions.SingleLine) == RegexOptions.SingleLine) && RegexMode == RegexMode.ECMAScript) {

throw new ArgumentException (“invalid Argument”);

}

        }

}

   public RegexMode RegexMode { get;

set {

if (((value & RegexMode.ECMAScript ) == RegexMode.ECMAScript) && RegexOptions == RegexOptions.SingleLine) {

throw new ArgumentException (“invalid Argument”);

}

        }

}

}

* Do not include ‘sentinel’ values in enums

Sentinel values are simply used to track the state of the enum itself, and have no actual meaning to the definition of the enum. A typical use might be to signal the ‘end’ of the enum, which could be used to identify where the last value in the enum is. If values are added, then this value could be changed, to represent the last of the additional values. Including sentinels confuses the meaning of the enum.

enum DeskTypes {

   Circular = 0x01,

   Oblong = 0x02.

   Rectangular = 0x03,

   LastValue = 0x03

}

Instead, check the values explicitly. Using range checks is valid, so long as all values within the range are also valid:

// CreateNew = 1, Append = 6, all interim values are defined

if (mode < FileMode.CreateNew || mode > FileMode.Append) {

throw new ArgumentException;

}

 

// check the supported values explicitly…

if (target == EnvironmentVariableTarget.Machine) {

   // take some action…

}

else if (target == EnvironmentVariableTarget.User) {

   // take some action…

}

else {

throw new ArgumentException;

}

* Do Not provide “Reserved” values, which are intended to be for future use

Because you can add values to your enum at a later stage, there is no reason to provide a reserved value, and including it will confuse the meaning of the enum.

Example:

enum ConsoleKeys {

       KeyA,

       KeyB, // …

       KeyOEM1,

       ReservedForFutureUse1,

       ReservedForFutureUse2,

// …

    }

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

 

Annotation (Brad Abrams)

 

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.

 

* Do use Int32 (the default in most programming languages) as the underlying type of an enum unless any of the following is true[2]:

1. The enum represents flags, and you expect many flags (>32) now or in the future.

Annotation (BradA): This may not be as uncommon a concern as you might first expect. We are only in V2.0 of the .NET Framework and we are already running out of values in the CodeDom GeneratorSupport enum. In retrospect, we should have used a different mechanism for communicating support of a larger underlying type.

2. The type needs to be different than Int32 for backward compatibility. (TODO: add an example)

3. You expect to it to be common to create a hundred or more instances of the enum by using the enum as a field in a frequently instantiated structure or class; storing many instances in arrays, files, etc. In such cases smaller is better. If you expect to the enum to be used mostly as a singleton for flow of control the size makes little differences.

Annotation (BradA): Note that for in memory usage you should be aware that managed objects are always DWORD aligned so you effectively need multiple enums or other small structures in an instance to pack a smaller enum with to make a difference as the total instance size is always going to be rounded up to a DWORD.

Annotation (BradA): Keep in mind it is a binary breaking change to change the size of the enum type once you have shipped, so chose wisely, with an eye on the future. Our experience is that Int32 is usually the right choice and thus we made it the default.

            Do not use any Enum suffix on enum types.

For example do not define:

public enum ColorEnum {Red, Green, Blue};

Instead use:
public enum Color {Red, Green, Blue};

 

* Do not publicly expose enums with one value only.

A common practice for ensuring future extensibility of C APIs is to add reserved parameters to method signatures. Such reserved parameters can be expressed as enums with a single default value. This should not be done in the .NET Framework APIs. Method overloading allows adding parameters in future releases.

// BAD DESIGN

public enum SomeOption {

   DefaultOption

   // we will add more options in the future

}

// The option parameter is not needed. It can always be added in the future

// to an overload of SomeMethod()

public void SomeMethod(SomeOption option) { … }

* Do not add the suffix ‘flags’ or ‘flag’ to an enum

The enum’s documentation should make it clear if it is a standard, or a flags enum.

Bad:

[Flags]

public enum ColorFlags { //…

Good:

[Flags]

public enum Colors { //

Adding Values to Enums

It is very common to need to add additional values to an enum after you have shipped the first version. There is a potential problem when the new value is returned from an existing API as poorly written applications may not handle the new value correctly. Documentation, samples and FxCop rules encourage application developers to write robust code per the guidelines described below. Therefore it is generally acceptable to add values to enums but as with any guideline there may be exceptions to the rule based on customer data.

* It is acceptable to add values to enums

If a caller receives an enum as an out or return value (or as a parameter to a virtual method), and switches over every valid value of the enum and throws unconditionally in the default case, then added values will cause the caller to perform the default case, and throw

If a caller receives an enum as an out or return value, and performs some ‘default’ behavior in the default case, then added values will behave as if they were default values

If you receive compatibility data for your application which indicates returning the new values from the existing API will cause issues for callers, consider adding a new API which returns the new (and old) values, and deprecate the old API. This will ensure your existing code remains compatible.

Annotation (Clemens Szyperski): Adding a value to an enum has a very real possibility of breaking a client. Before the addition of the new enum value, a client who was throwing unconditionally in the default case presumably never actually threw the exception, and the corresponding catch path is likely untested. Now that the new enum value can pop up, the client will throw and likely fold.

The biggest concern with adding values to enums, is that you don't know whether clients perform an exhaustive switch over an enum or a progressive case analysis across wider-spread code. Even with the FxCop rules above in place and even when it is assumed that client apps pass FxCop without warnings, we still would not know about code that performs things like if (myEnum == someValue) ... in various places.

Clients may instead perform point-wise case analyses across their code, resulting in fragility under enum versioning. It is important to provide specific guidelines to developers of enum client code detailing what they need to do to survive the addition of new elements to enums they use. Developing with the suspected future versioning of an enum in mind is the required attitude.