Enum Switches

I was at a code review for one of the developers when I came across the following lines of code:

public void DoSomething(int value)
{
switch (value)
{
case (int)SomeEnum.Value1:
DoThis();
break;
case (int)SomeEnum.Value2:
DoThat();
break;
}
}
The intention behind these lines was getting a value that was a value of the enum without having to expose the enum itself. In this case, the value was given by direct input from the user.


It might be obvious to some of you, but this code section has a few flaws. First of all, there is no check for the value and there is a conversion to the integer type for every case statement.


So I dug into the IL and found that one of these things was not even a problem: The code is compiled so that the values of the enum are translated to strongly typed integer values. This means that it doesn’t really matter, IL-wise, if you convert the value in the switch to the enum or each value to integer, it comes out the same.


What did bother me was the fact that this programmer didn’t check if the value sent to the method was even a valid value. This is done by using the Enum.IsDefined method and checks that the value passed is a defined value in the enum’s range of values.

Advertisements

5 thoughts on “Enum Switches

  1. ::What did bother me was the fact that this
    ::programmer didn’t check if the value sent to
    ::the method was even a valid value.

    Why should he? No switch will hit, and the enum will fall through. It all depends on whether he wnts to get an exception. EVEN then he may be faster not usind Enum.IsDefined but put the exception into the default fallthrough for the switch.

    The switch perfectly handles this situation.

  2. Short answer to clearly show int conversion bug:

    enum SomeEnum: long
    {
    Value1 = 111111111L,
    Value2 = 4406078407L
    }

    ;o))

  3. Thomas,

    If I want to get an exception for a value that’s not in the enum, using the default clause is bad. Bad for versioning. If I add a value to the list of values in the enum, I will have to find that code and add a case clause for that enum value.
    In this case, a valid enum value is not erroneous input.

    AT,

    Yeah, that’s one way of flunking it. :D

    Kevin, Jason,

    There is a performance penalty, I’ll grant you that, but there’s no better way of doing this. What Brad is talking about in his post is the need to get a valid value for a certain API function. Using IsDefined then is not right, since the enum values are changeable. However, in this case, adding a check for IsDefined, removes the need to check for it again in the cases across versions, since there is and will be no such thing as an invalid enum member.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s