Extending Enums, Overkill?

Question

I have an object that needs to be serialized to an EDI format. For this example we'll say it's a car. A car might not be the best example b/c options change over time, but for the real object the Enums will never change.

I have many Enums like the following with custom attributes applied.

public enum RoofStyle
{
    [DisplayText("Glass Top")]
    [StringValue("GTR")]
    Glass,
    [DisplayText("Convertible Soft Top")]
    [StringValue("CST")]
    ConvertibleSoft,
    [DisplayText("Hard Top")]
    [StringValue("HT ")]
    HardTop,
    [DisplayText("Targa Top")]
    [StringValue("TT ")]
    Targa,
}

The Attributes are accessed via Extension methods:

public static string GetStringValue(this Enum value)
{
    // Get the type
    Type type = value.GetType();

    // Get fieldinfo for this type
    FieldInfo fieldInfo = type.GetField(value.ToString());

    // Get the stringvalue attributes
    StringValueAttribute[] attribs = fieldInfo.GetCustomAttributes(
        typeof(StringValueAttribute), false) as StringValueAttribute[];

    // Return the first if there was a match.
    return attribs.Length > 0 ? attribs[0].StringValue : null;
}

public static string GetDisplayText(this Enum value)
{
    // Get the type
    Type type = value.GetType();

    // Get fieldinfo for this type
    FieldInfo fieldInfo = type.GetField(value.ToString());

    // Get the DisplayText attributes
    DisplayTextAttribute[] attribs = fieldInfo.GetCustomAttributes(
        typeof(DisplayTextAttribute), false) as DisplayTextAttribute[];

    // Return the first if there was a match.
    return attribs.Length > 0 ? attribs[0].DisplayText : value.ToString();
}

There is a custom EDI serializer that serializes based on the StringValue attributes like so:

    StringBuilder sb = new StringBuilder();
    sb.Append(car.RoofStyle.GetStringValue());
    sb.Append(car.TireSize.GetStringValue());
    sb.Append(car.Model.GetStringValue());
    ...

There is another method that can get Enum Value from StringValue for Deserialization:

   car.RoofStyle = Enums.GetCode<RoofStyle>(EDIString.Substring(4, 3))

Defined as:

public static class Enums
    {
        public static T GetCode<T>(string value)
        {
            foreach (object o in System.Enum.GetValues(typeof(T)))
            {
                if (((Enum)o).GetStringValue() == value.ToUpper())
                    return (T)o;
            }
            throw new ArgumentException("No code exists for type " + typeof(T).ToString() + " corresponding to value of " + value);
        }
} 

And Finally, for the UI, the GetDisplayText() is used to show the user friendly text.

What do you think? Overkill? Is there a better way? or Goldie Locks (just right)?

Just want to get feedback before I intergrate it into my personal framework permanently. Thanks.

Answer 1

Can't say I've ever seen it done that way but the consumer code is relatively simple so I'd probably enjoy using it.

The only thing that sticks out for me is the potential for consumers having to deal with nulls - which might be able to be removed. If you have control over the attributes (which you do, from the sounds of it), then there should never be a case where GetDisplayText or GetStringValue return null so you can remove

return attribs.Length > 0 ? attribs[0].StringValue : null;

and replace it with

return attribs[0].StringValue;

in order to simplify the interface for consumer code.

Answer 2

Personally, I think you are abusing the language and trying to use enums in a way they were never intended. I would create a static class RoofStyle, and create a simple struct RoofType, and use an instance for each of your enum values.

Answer 3

I don't see a problem with it - actually, I do the same. By this, I achieve verbosity with the enum, and can define how the enum is to be translated when I use it to request data, eg. RequestTarget.Character will result in "char".

Answer 4

The part you're using to serialize is fine. The deserialization part is awkwardly written. The main problem is that you're using ToUpper() to compare strings, which is easily broken (think globalization). Such comparisons should be done with string.Compare instead, or the string.Equals overload that takes a StringComparison.

The other thing is that performing these lookups again and again during deserialization is going to pretty slow. If you're serializing a lot of data, this could actually be quite noticeable. In that case, you'd want to build a map from the StringValue to the enum itself - throw it into a static Dictionary<string, RoofStyle> and use it as a lookup for the round-trip. In other words:

public static class Enums
{
    private static Dictionary<string, RoofStyle> roofStyles =
        new Dictionary<string, RoofStyle>()
    {
        { "GTR", RoofStyle.Glass },
        { "CST", RoofStyle.ConvertibleSoft },
        { "HT ", RoofStyle.HardTop },
        { "TT ", RoofStyle.TargaTop }
    }

    public static RoofStyle GetRoofStyle(string code)
    {
        RoofStyle result;
        if (roofStyles.TryGetValue(code, out result))
            return result;
        throw new ArgumentException(...);
    }
}

It's not as "generic" but it's way more efficient. If you don't like the duplication of string values then extract the codes as constants in a separate class.

If you really need it to be totally generic and work for any enum, you can always lazy-load the dictionary of values (generate it using the extension methods you've written) the first time a conversion is done. It's very simple to do that:

static Dictionary<string, T> CreateEnumLookup<T>()
{
    return Enum.GetValues(typeof(T)).ToDictionary(o => ((Enum)o).GetStringValue(),
        o => (T)o);
}

P.S. Minor detail but you might want to consider using Attribute.GetCustomAttribute instead of MemberInfo.GetCustomAttributes if you only expect there to be one attribute. There's no reason for all the array fiddling when you only need one item.

Answer 5

Why don't you create a type with static members such as mikerobi said

Example...

public class RoofStyle
{
    private RoofStyle() { }
    public string Display { get; private set; }
    public string Value { get; private set; }

    public readonly static RoofStyle Glass = new RoofStyle
    {
        Display = "Glass Top",  Value = "GTR",
    };
    public readonly static RoofStyle ConvertibleSoft = new RoofStyle
    {
        Display = "Convertible Soft Top", Value = "CST",
    };
    public readonly static RoofStyle HardTop = new RoofStyle
    {
        Display = "Hard Top", Value = "HT ",
    };
    public readonly static RoofStyle Targa = new RoofStyle
    {
        Display = "Targa Top", Value = "TT ",
    };
}

BTW...

When compiled into IL an Enum is very similar to this class structure.

... Enum backing fields ...

.field public specialname rtspecialname int32 value__
.field public static literal valuetype A.ERoofStyle Glass = int32(0x00)
.field public static literal valuetype A.ERoofStyle ConvertibleSoft = int32(0x01)
.field public static literal valuetype A.ERoofStyle HardTop = int32(0x02)
.field public static literal valuetype A.ERoofStyle Targa = int32(0x03)

... Class backing fields ...

.field public static initonly class A.RoofStyle Glass
.field public static initonly class A.RoofStyle ConvertibleSoft
.field public static initonly class A.RoofStyle HardTop
.field public static initonly class A.RoofStyle Targa

Answer 6

IMHO, the design is solid, and will work. However, reflection tends to be a litle slow, so if those methods are used in tight loops, it might slow down the whole application.

You could try caching the the return values into a Dictionary<RoofStyle, string> so they are only reflected once, and then fetched from cache.

Something like this:

    private static Dictionary<Enum, string> stringValues 
      = new Dictionary<Enum,string>();

    public static string GetStringValue(this Enum value)
    {
        if (!stringValues.ContainsKey(value))
        {
            Type type = value.GetType();
            FieldInfo fieldInfo = type.GetField(value.ToString());
            StringValueAttribute[] attribs = fieldInfo.GetCustomAttributes(
                typeof(StringValueAttribute), false) as StringValueAttribute[];
            stringValues.Add(value, attribs.Length > 0 ? attribs[0].StringValue : null);
        }
        return stringValues[value];
    }