C#: Enum anti-patterns

问题:

There has been talk of Enums in general violating Clean Code-principles, so I'm looking for people's favorite Enum anti-patterns and alternative solutions for these.

For example I've seen code like this:

switch(enumValue) {
    case myEnum.Value1:
        // ...
        break;
    case myEnum.Value2:
        // ...
        break;
}

It's one step better than switch-statements with magic strings, but this probably could have been solved better with a factory, a container or other pattern.

Or even old-school code like this:

if(enumValue == myEnum.Value1) {
   // ...
} else if (enumValue == myEnum.Value2) {
   // ...
}

What other anti-patterns and better implementations have you experienced with enums?

回答1:

I think Enums are quite useful. I've written a few extensions for Enum that have added even more value to its use

First, there's the Description extension method

public static class EnumExtensions
{
    public static string Description(this Enum value)
    {
        var entries = value.ToString().Split(ENUM_SEPERATOR_CHARACTER);
        var description = new string[entries.Length];
        for (var i = 0; i < entries.Length; i++)
        {
            var fieldInfo = value.GetType().GetField(entries[i].Trim());
            var attributes = (DescriptionAttribute[])fieldInfo.GetCustomAttributes(typeof(DescriptionAttribute), false);
            description[i] = (attributes.Length > 0) ? attributes[0].Description : entries[i].Trim();
        }
        return String.Join(", ", description);
    }
    private const char ENUM_SEPERATOR_CHARACTER = ',';
}

This will allow me to define en enum like this:

 public enum MeasurementUnitType
 {
    [Description("px")]
    Pixels = 0,
    [Description("em")]
    Em = 1,
    [Description("%")]
    Percent = 2,
    [Description("pt")]
    Points = 3
 }

And get the label by doing this: var myLabel = rectangle.widthunit.Description() (eliminating any need for a switch statement).

This will btw return "px" if rectangle.widthunit = MeasurementUnitType.Pixels or it will return "px,em" if rectangle.widthunit = MeasurementUnitType.Pixels | MeasurementUnitType.Em.

Then, there is a

    public static IEnumerable<int> GetIntBasedEnumMembers(Type @enum)
    {
        foreach (FieldInfo fi in @enum.GetFields(BindingFlags.Public | BindingFlags.Static))
            yield return (int)fi.GetRawConstantValue();
    }

Which will let me traverse any enum with int based values and return the int values themselves.

I find these to be very useful in an allready useful concept.

回答2:

I see having two switch statements as a symptom of non-OO design as explained further in this answer.

回答3:

This isn't an answer, as much as contributing to a list of Enum anti-patterns.

During a code review this morning, I ran into a case similar to the following, all in the same class.

Two cases:

1. Before drinking
2. After drinking

..

    public enum ListEnum 
    { 
        CategoryOne,
        CategoryTwo,
        CategoryThree,
        CategoryFour
    }


    public class UIELementType
    {
        public const string FactoryDomain = "FactoryDomain";
        public const string Attributes = "Attributes";
    }

回答4:

Using enums in not anti-pattern. In some books about refactoring this code is used to demonstrate how to replace it with polymorphism. It would be OK when you overuse enums in code.

回答5:

It all depends what your trying to do with the enum.

1. If you are trying to stop your developers from passing magic numbers into your operations and you want to keep the data referential integrity intact with your DB then, YES! Use T4-Templates (using your ORM) to go to your MeasurementUnitTypes table and generate a enum with the ID, Name and Description columns matching the enum’ int, Enum_Name and Description Attribute (nice approach for additional field\data to enum @danijels) as suggested above. If you add a new Measurement Type to your MeasurementUnitTypes table you can just right click and run the T4-Template and the enum code is generated for that new row added in the table. I don’t like hard-coded data in my application that doesnt link to my DB hence the mention of the T4-Template approach. It is not extensible otherwise...what if some other external system wants to retrieve our Measurement Criteria used in our system, then it is hard-coded in the system and you can't expose it to the client via a service. That left there.

2. If the purpose is not data related and you have some logic assigned to a specific enum then NO! this violates the SOLID (Open close principle) as you would somewhere in your application apply a switch or bunch of Ifs to action the logic per enum, ALSO if you did it REALLY bad these switches or Ifs are all over the show....good luck adding a new enum... so it is not open for extension and closed for modification as you need to modify existing code, as per the SOLID principle.

   If your choice is 2 then I suggest then to replace your enum with the following using the example from @danijels comment:

   public interface IMeasurementUnitType
   {
       int ID { get; }
   
       string Description { get; }
   
       // Just added to simulate a action needed in the system
       string GetPrintMessage(int size);
   }
   

The above code defines the interface (code contract) that each measurement should adhere to. Now lets define Percentage and Pixel measurement :

    public class PixelsMeasurementUnitType : IMeasurementUnitType
    {
        public int ID => 1;

        public string Description => "Pixel";

        public string GetPrintMessage(int size)
        {
            return $"This is a {Description} Measurement that is equal to {size} pixels of the total screen size";
        }
    }

    public class PercentMeasurementUnitType : IMeasurementUnitType
    {
        public int ID => 2;

        public string Description => "Persentage";

        public string GetPrintMessage(int size)
        {
            return $"This is a {Description} Measurement that is equal to {size} persent of total screen size (100)";
        }
    }

So wee have defined two types, we would use them in code as follows:

    var listOfMeasurmentTypes = AppDomain.CurrentDomain.GetAssemblies()
                        .SelectMany(s => s.GetTypes())
                        .Where(p => typeof(IMeasurementUnitType).IsAssignableFrom(p) 
                                    && !p.IsInterface)
                        .ToList();

Here we grab all the TYPES that extends the IMeasurementUnitType interface and NOT the interface itself. Now we can use the Activator to create instances of the classes to populate our UI controls:

    public IEnumerable<IMeasurementUnitType> GetInstantiatedClassesFromTypes(List<Type> types)
    {
        foreach (var type in types)
        {
            yield return (IMeasurementUnitType)Activator.CreateInstance(type);
        }
    }

You can change the code above to be generic for any type, AND NOW life happens and the client give a new measuring unit type called Point as a new requirement, I don't need to CHANGE ANY code, just add the new type (extend the code NOT modify). The new type will automatically be picked up in the application.

    public class PointMeasurementUnitType : IMeasurementUnitType
    {
        public int ID => 3;

        public string Description => "Point";

        public string GetPrintMessage(int size)
        {
            return $"This is a {Description} Measurement that is equal to {size} points of total screen size";
        }
    }

a Good idea would be to cache your types for performance benefits upon starting your application or try and use a DI container of your choice.

Also, one can argue that somewhere in you application you would need to distinguish between types and I agree, however you want to keep apples with apples. So try as far as possible to apply the same principle used for this types. If this type is used in some sort of Graphics processor (for example) class then have a IGraphicsProcessor and have your concrete classes that differentiate between these types for example PersentageAndPixelGraphicsProcessor (that extends from IGraphicsProcessor) or if it distinguishes only one type call it PersentageGraphicsProcessor.

Sorry for the HUGE SA but I really like enum's however I feel when you trying to separate logic using a enums it is a STRONG anti-pattern.

comments welcome,