Enum.Parse() or Switch

For converting a string to an enum, which of the following ways is better?

1. This code:

   colorEnum color = (colorEnum)Enum.Parse(typeof(colorEnum), "Green");
   

2. or this:

   string colorString = ...
   colorEnum color;        
   switch (colorString)
   {
       case "Green":
           color = colorEnum.Green;
           break;
       case "Red":
     开发者_开发知识库      color = colorEnum.Red;
           break;
       case "Orange":
           color = colorEnum.Orange;
           break;
       ....
   }
   

You should use the Enum.TryParse, if it fails you can handle the error correctly.

sample:

     ColorsEnum colorValue; 
     if (Enum.TryParse(colorString, out colorValue))        
        if (Enum.IsDefined(typeof(Colors), colorValue) | colorValue.ToString().Contains(","))  
           Console.WriteLine("Converted '{0}' to {1}.", colorString, colorValue.ToString());
        else
           Console.WriteLine("{0} is not an underlying value of the Colors enumeration.", colorString);
     else
        Console.WriteLine("{0} is not a member of the Colors enumeration.", colorString);

(Warning: includes plug for my own open source library...)

Personally I'd use Unconstrained Melody, which ends up with cleaner and more typesafe code:

ColorEnum color = Enums.ParseName<ColorEnum>(text);

You can use TryParseName if you suspect it may be invalid. Obviously this requires an extra library, but hopefully you'll find other things in there helpful too :)

Enum.TryParse from .NET 4 is better than the other built-in options, but:

• You won't catch non-enum types at compile time, e.g. Enum.TryParse<int>(...) will still compile; Unconstrained Melody really only allows enum types
• Enum.TryParse will also parse "1" (or whatever the numeric value is when converted to a string) - if you really only expect names, I think it's better to only accept names

I definitely wouldn't switch on the string values - it means if you rename the enum values, you've got to remember to rename the case value as well.

And what about Enum.TryParse<TEnum> ?

string myColorStr = "red";
colorEnum myColor;
if(!Enum.TryParse<colorEnum>(myColorStr, true, out myColor))
{
    throw new InvalidOperationException("Unknown color " + myColorStr);
}

Number 1 simply on readability and maintainability. If you extend the enum you need to do no extra work, wheras with 2 you have to add more cases to the switch statement

Because you added the tag 'performance', I'm going to go with the switch.
Yes, you will have to change the cases when you rename/add/remove anything in the enum. Well that's just too bad then. Any variant of Enum.Parse/TryParse uses a lot of weird code and some reflection, just take a look inside the function with ILSpy or such. Then there is also the issue of accepting "-12354" and even a comma-separated list of valid names (resulting in all of them ORed together) even when the enum doesn't have a [Flags] attribute.

As an alternative, you could make a dictionary that translates enum names to values. It should actually be faster than the switch, because a switch on strings also goes through a dictionary but you save the actual switch part.

Obviously both ways cost some more maintenance than enum.parse and variants; whether it's worth it is up to you, since out of all of us only you have enough knowledge of the project to make the performance/coding-time trade off.

1) Is much better. It's cleaner code. You are doing in one line what would take multiple in 2). Also, it's less bug prone. When you add another item to colorEnum, you would need to remember to extend 2) wheras 1) would just work.

You may also want some error handling on the Enum.Parse.

Other than the fact that the two different code snippets doesn't do the same thing, I'd use this:

colorEnum color;
if (!colorEnum.TryParse(colorString, true, out color)
    color = colorEnum.Green;    // Or whatever default value you wish to have.

If you don't have .NET 4.0 then I'd do something like this:

public static TEnum ToEnum<TEnum>(this string strEnumValue, TEnum defaultValue)
{
    if (!Enum.IsDefined(typeof(TEnum), strEnumValue))
        return defaultValue;

    return (TEnum)Enum.Parse(typeof(TEnum), strEnumValue);
}

This is an Extension Method to string.

Personally, while I am totally fine with the Enum.Parse solution for non-performance scenarios (read: one off run of this function occasionally ... and there are many such scenarios to be sure), I can't countenance the thought of possibly involving some reflection type method when this function needs to be performed in a loop over hundreds/thousands plus enum values at once. Gack!

So the following is a solution that gets some of the best of both worlds.

Just retrieve all values of the enum at startup time or what not, whenever it works best for you (below is one way of doing that), and then construct a Dictionary with them.

    private static Dictionary<string, Color> colorDictionary;
    public static Dictionary<string, Color> ColorDictionary
    {
        get
        {
            if (colorDictionary== null) {
                colorDictionary = new Dictionary<string, Color>();
                var all = Enum.GetValues(typeof(Color)).OfType<Color>();
                foreach (var val in all)
                    dict.Add(val.ToString(), val);
            }
            return colorDictionary;
        }
    }

I find the switch variant horrible since you will have to modify the switch everytime you change the enum also.

I like to use the TryParse that belongs to your enum. So you can use it like this

string colorString = .....
colorEnum color;

colorEnum.TryParse(colorString, out color);

Or if you don't care about the case of the string

colorEnum.TryParse(colorString, true, out color);

The returnvalue of TryParse is true if the string was a valid enum, false if not.

On a performance point of view, as enums are implemented as static fields, the parse method will probably ends up doing refection on then enum type and try a GetField method which might be faster than the case. On the other end, if 90% of the case, the color is green, the case will be very fast... Note that the CLR sometimes rearrange code internally, changing the order of the case based on statistic (actually, I'm not sure it does that but the doc claims it could).

I use the following, it gets you all the type safety while still not falling over when you add new values into the Enum, it's also very fast.

public static colorEnum? GetColorFromString(string colorString)
{
    colorEnum? retVal = null;
    if(Enum.IsDefined(typeof(colorEnum), colorString))
        retVal = (colorEnum)Enum.Parse(typeof(colorEnum), colorString);
    return retVal;
}

My test with 8 items in the enum shows this way to be faster than the switch method.

Or else you can use (the very slow way):

public static colorEnum? GetColorFromString(string colorString)
{
    foreach (colorEnum col in Enum.GetValues(typeof(colorEnum)))
    {
        if (col.ToString().Equals(colorString))
        {
            return col;
        }
    }
    return null;
}