36

There is something that I cannot understand in C#. You can cast an out-of-range int into an enum and the compiler does not flinch. Imagine this enum:

enum Colour
{
    Red = 1,
    Green = 2,
    Blue = 3
}

Now, if you write:

Colour eco;
eco = (Colour)17;

The compiler thinks that’s fine. And the runtime, too. Uh?

Why did the C# team decide to make this possible? This decision misses the point of using enums, I think, in scenarios like this:

void DoSomethingWithColour(Colour eco)
{
    //do something to eco.
}

In a strong-typed language like C#, I would like to assume that eco will always hold a legal Colour value. But this is not the case. A programmer could call my method with a value of 17 assigned to eco (as in previous code snippet), so the code in my method must not assume that eco holds a legal Colour value. I need to test for it explicitly and handle the exceptional values as I please. Why is this?

In my humble opinion, it would be much nicer if the compiler issued an error (or even a warning) message when casting an out-of range int into an enum, if the int value is known at compile time. If not, the runtime should throw an exception at the assignment statement.

What do you think? Is there any reason why this is so?

(Note. This is a question I posted ages ago on my blog but got no informative response.)

casperOne
  • 73,706
  • 19
  • 184
  • 253
CesarGon
  • 15,099
  • 6
  • 57
  • 85

12 Answers12

32

Guessing about 'why' is always dangerous, but consider this:

enum Direction { North =1, East = 2, South = 4, West = 8 }
Direction ne = Direction.North | Direction.East;

int value = (int) ne; // value == 3
string text = ne.ToString();  // text == "3"

When the [Flags] attribute is put in front of the enum, that last line changes to

string text = ne.ToString();  // text == "North, East"
H H
  • 263,252
  • 30
  • 330
  • 514
  • Your point being? :-) If you want to make SouthEast to be a valid direction, I think you should include it in the enum declaration as SouthEast = 10, for example. Agree? – CesarGon Nov 18 '09 at 19:14
  • 2
    They are called flag enumerations. Very vaild design. – Matthew Whited Nov 18 '09 at 19:15
  • 20
    No, do not agree. Bit fields are a perfectly valid use of an enum, so much so that the [Flags] attribute is not even required to use them as such. – Ed S. Nov 18 '09 at 19:16
  • In this example you could certainly add a SouthEast value, but what if your enum specifies a bunch of options? Are you going to create one value for every possible combination? – Ed S. Nov 18 '09 at 19:17
  • 5
    @CesarGon... no. If I want to set flags on a property, I don't want to have to list every possible combination of options. For directions this might be trivial to do, but in most other cases it would suck. – Brandon Nov 18 '09 at 19:17
  • No, don't agree. enums are useful (intended) to be used as bitmasks (sets). The example is weak in that North|South isn't a valid direction, but in general the number of combinations would be far to big. – H H Nov 18 '09 at 19:18
  • 2
    @CesarGon: Imagine your flags enum contained not 4 bits but 16 bits. Would you really like to define all 65536 possible combinations (assuming all combinations are valid)? The point of the [flags] attribute is just that you don't have to do that. (Your points stays valid, though: You could still cast (Direction)16, although 16 is not a valid Direction) – Niki Nov 18 '09 at 19:18
  • 5
    Yes, flag enums are a valid design, no problem with that. But I am afraid that the consequence is that enum types are providing less value (in terms of type safety) than they could. Maybe language designers should treat enum types as one thing and bitfields (called flag enums in C#) as a different thing, using different constructs in the language. I would like to have the type safety that enum types are supposed to give you, and the flexibility that bitfields are supposed to offer. – CesarGon Nov 18 '09 at 19:21
  • You could alway use "static readonly" or "const" if you don't want other values. – Matthew Whited Nov 18 '09 at 19:22
  • Sorry folks, I wasn't too clear in my previous comments: I did not mean to say that you should add every single combination of valies to a flag enum; flag enums (bitfields) are meant to be combined as necessary. I was referring to non-flag enums only (see my previous comment please). – CesarGon Nov 18 '09 at 19:23
  • @Matthew Whited: Could you please provide an example of "static readonly" or "const" on an enum? – CesarGon Nov 18 '09 at 19:27
15

Not sure about why, but I recently found this "feature" incredibly useful. I wrote something like this the other day

// a simple enum
public enum TransmissionStatus
{
    Success = 0,
    Failure = 1,
    Error = 2,
}
// a consumer of enum
public class MyClass 
{
    public void ProcessTransmissionStatus (TransmissionStatus status)
    {
        ...
        // an exhaustive switch statement, but only if
        // enum remains the same
        switch (status)
        {
            case TransmissionStatus.Success: ... break;
            case TransmissionStatus.Failure: ... break;
            case TransmissionStatus.Error: ... break;
            // should never be called, unless enum is 
            // extended - which is entirely possible!
            // remember, code defensively! future proof!
            default:
                throw new NotSupportedException ();
                break;
        }
        ...
    }
}

question is, how do I test that last case clause? It is completely reasonable to assume someone may extend TransmissionStatus and not update its consumers, like poor little MyClass above. Yet, I would still like to verify its behaviour in this scenario. One way is to use casting, such as

[Test]
[ExpectedException (typeof (NotSupportedException))]
public void Test_ProcessTransmissionStatus_ExtendedEnum ()
{
    MyClass myClass = new MyClass ();
    myClass.ProcessTransmissionStatus ((TransmissionStatus)(10));
}
johnny g
  • 3,533
  • 1
  • 25
  • 40
  • 1
    Are you serious? I find that a quick & dirty hack, with all due respect. :-) – CesarGon Nov 18 '09 at 19:33
  • 4
    So ... presuming enums were completely type safe with value validation, how would you go about testing the preceding conditional? Would you rather add an "Unsupported" value into every enum you ever write for the express purpose of testing default clauses? If you ask me, that smells like a much bigger stink ;) – johnny g Nov 18 '09 at 19:53
  • 1
    In fact, an Unsupported value in an enum is explicitly clear. I think that would be a much better way to do the testing, yes. :-) – CesarGon Nov 18 '09 at 19:57
  • That's quite nifty, Johnny! I'll remember that! (+1) – leifericf Nov 04 '10 at 00:59
  • Is it not better if the language won't allow you to cast an 'invalid' value to enum and handle cause error later? – Devs love ZenUML Sep 13 '17 at 01:43
5

I certainly see Cesar's point, and I remember this initially confused me too. In my opinion enums, in their current implementation, are indeed a little too low level and leaky. It seems to me that there would be two solutions to the problem.

1) Only allow arbitrary values to be stored in an enum if its definition had the FlagsAttribute. This way, we can continue to use them for a bitmask when appropriate (and explicitly declared), but when used simply as placeholders for constants we would get the value checked at runtime.

2) Introduce a separate primitive type called say, bitmask, that would allow for any ulong value. Again, we restrict standard enums to declared values only. This would have the added benefit of allowing the compiler to assign the bit values for you. So this:

[Flags]
enum MyBitmask
{ 
    FirstValue = 1, SecondValue = 2, ThirdValue = 4, FourthValue = 8 
}

would be equivalent to this:

bitmask MyBitmask
{
    FirstValue, SecondValue, ThirdValue, FourthValue 
}

After all, the values for any bitmask are completely predictable, right? As a programmer, I am more than happy to have this detail abstracted away.

Still, too late now, I guess we're stuck with the current implementation forever. :/

Mike Chamberlain
  • 39,692
  • 27
  • 110
  • 158
  • Thanks, Mikey. That's what I mean when I say that *real* enums and bitfields ("flag" enums) are in fact two very different kinds of things. C++ and related languages have trated them as the same thing, and I am afraid that C# has inherited that flaw. But the semantics are clearly different. Maybe Eric Lippert or Jon Skeet might drop by and shed some light on this issue. – CesarGon Nov 04 '10 at 00:02
4

You don't need to deal with exceptions. The precondition for the method is that callers should use the enum, not cast any willy nilly int to the said enum. That would be madness. Isn't the point of enums not to use the ints?

Any dev who would cast 17 to the Colour enum would need 17 kicks up their backside as far as I'm concerned.

Wim
  • 11,998
  • 1
  • 34
  • 57
  • Agreed about the kicks. :-D But defensive programming is a good practice. And when you're doing public interfaces for your class library, you *know* you should check input parameters. – CesarGon Nov 18 '09 at 19:17
  • So you set a default value on a switch and either throw an exception or break out to the debugger. There are plenty of vaild cases for these integer values from enumerations. – Matthew Whited Nov 18 '09 at 19:24
  • @CesarGon: So check them yourself, in the example you give all you need to check is that the value is between 1 and 3. CmdrTallen has a comment showing how to check in general. – tloach Nov 18 '09 at 19:25
  • 3
    Exactly, that is what I am doing. I check for preconditions myself at method entry. My point is, this shouldn't be necessary. My argument is: when you specify a parameter as int, you don't need to check that the received value is an int (the compiler/runtime guarantee that). When you specify a parameter as a class FooBar, same thing: you don't need to check. However, when you specify a parameter as a Colour enum, you still have to check that it is a Colour enum. I find this treatment of enums a bit inconsistent. – CesarGon Nov 18 '09 at 19:30
4

That is one of the many reasons why you should never be assigning integer values to your enums. If they have important values that need to be used in other parts of the code turn the enum into an object.

Bryan Rowe
  • 9,185
  • 4
  • 26
  • 34
  • 1
    There are also plenty of reasons to include the values. You could be interoperating with an external system and just using the enumeration to make your code cleaner. – Matthew Whited Nov 18 '09 at 19:25
  • 2
    I dont think there are 'plenty' of reasons. I understand the argument of interfacing with external systems, sure. But there is no way you can argue that an enum with values assigned is going to result in cleaner code -- more like smellier code. You can follow a simple value object pattern and create easy to understand objects... and, you know, follow the whole OOP thing? – Bryan Rowe Nov 18 '09 at 19:34
  • 3
    When transferring data to and from the DB you need to coerce enums from int to enum and back. – Guy Jan 04 '10 at 20:26
4

That is unexpected... what we really want is to control the casting... for instance:

Colour eco;
if(Enum.TryParse("17", out eco)) //Parse successfully??
{
  var isValid = Enum.GetValues(typeof (Colour)).Cast<Colour>().Contains(eco);
  if(isValid)
  {
     //It is really a valid Enum Colour. Here is safe!
  }
}
Jaider
  • 14,268
  • 5
  • 75
  • 82
3

When you define an enum you are essentially giving names to values (syntatic sugar if you will). When you cast 17 to Colour, you are storing a value for a Colour that has no name. As you probably know in the end it is just an int field anyways.

This is akin to declaring an integer that would accept only values from 1 to 100; the only language I ever saw that supported this level of checking was CHILL.

Otávio Décio
  • 73,752
  • 17
  • 161
  • 228
  • Disagree. Maybe in the olden days of C++ it was syntactic sugar. But in .NET, enums are first-class types. That's the whole point of having enum types! – CesarGon Nov 18 '09 at 19:18
  • Can you imagine the overhead for the CLR to check for valid enums? I think in the end they made the decision to let it go unchecked because it was not worth checking it. – Otávio Décio Nov 18 '09 at 19:20
  • How about the fact you could no longer use flag enumerations? – Matthew Whited Nov 18 '09 at 19:29
  • As I said in other comment, maybe the language and runtime should treat enum types and bitfields (flag enums) as different constructs. – CesarGon Nov 18 '09 at 19:31
  • 1
    FYI Pascal (and by extension Delphi) allows this subclassing of primitive scalar types too. As in: type TsomeType = 2..12; – Pete Stensønes Jul 18 '12 at 14:50
3
if (!Enum.IsDefined(typeof(Colour), 17))
{
    // Do something
}
Jon Schneider
  • 25,758
  • 23
  • 142
  • 170
1

Short version:

Don't do this.

Trying to change enums into ints with only allowing valid values (maybe a default fallback) requires helper methods. At that point you have don't have an enum--you reallly have a class.

Doubly so if the ints are improtant--like Bryan Rowe said.

Broam
  • 4,602
  • 1
  • 23
  • 38
0

Old question, but this confused me recently, and Google brought me here. I found an article with further searching that finally made it click for me, and thought I'd come back and share.

The gist is that Enum is a struct, which means it's a value type (source). But in essence it acts as a derived type of the underlying type (int, byte, long, etc.). So if you can think of an Enum type as really just the same thing as its underlying type with some added functionality/syntactic sugar (as Otávio said) then you'll be aware of this "problem" and be able to protect against it.

Speaking of that, here is the heart of an extension method I wrote to easily convert/parse things to an Enum:

if (value != null)
{
    TEnum result;
    if (Enum.TryParse(value.ToString(), true, out result))
    {
        // since an out-of-range int can be cast to TEnum, double-check that result is valid
        if (Enum.IsDefined(typeof(TEnum), result.ToString()))
        {
            return result;
        }
    }
}

// deal with null and defaults...

The variable value there is of type object, as this extension has "overloads" that accept int, int?, string, Enum, and Enum?. They're all boxed and sent to the private method that does the parsing. By using both TryParse and IsDefined in that order, I can parse all the types just mentioned, including mixed case strings, and it's pretty robust.

Usage is like so (assumes NUnit):

[Test]
public void MultipleInputTypeSample()
{
    int source;
    SampleEnum result;

    // valid int value
    source = 0;
    result = source.ParseToEnum<SampleEnum>();
    Assert.That(result, Is.EqualTo(SampleEnum.Value1));

    // out of range int value
    source = 15;
    Assert.Throws<ArgumentException>(() => source.ParseToEnum<SampleEnum>());

    // out of range int with default provided
    source = 30;
    result = source.ParseToEnum<SampleEnum>(SampleEnum.Value2);
    Assert.That(result, Is.EqualTo(SampleEnum.Value2));
}

private enum SampleEnum
{
    Value1,
    Value2
}

Hope that helps somebody.

Disclaimer: we do not use flags/bitmask enums... this has not been tested with that usage scenario and probably wouldn't work.

sliderhouserules
  • 3,415
  • 24
  • 32
0

Thought I'd share the code I ended up using to validate Enums, as so far we don't seem to have anything here that works...

public static class EnumHelper<T>
{
    public static bool IsValidValue(int value)
    {
        try
        {
            Parse(value.ToString());
        }
        catch
        {
            return false;
        }

        return true;
    }

    public static T Parse(string value)
    {
        var values = GetValues();
        int valueAsInt;
        var isInteger = Int32.TryParse(value, out valueAsInt);
        if(!values.Select(v => v.ToString()).Contains(value)
            && (!isInteger || !values.Select(v => Convert.ToInt32(v)).Contains(valueAsInt)))
        {
            throw new ArgumentException("Value '" + value + "' is not a valid value for " + typeof(T));
        }

        return (T)Enum.Parse(typeof(T), value);
    }

    public static bool TryParse(string value, out T p)
    {
        try
        {
            p = Parse(value);
            return true;
        }
        catch (Exception)
        {
            p = default(T);
            return false;
        }

    }

    public static IEnumerable<T> GetValues()
    {
        return Enum.GetValues(typeof (T)).Cast<T>();
    }
}
Mike Chamberlain
  • 39,692
  • 27
  • 110
  • 158
-1

It would throw an error by using the Enum.Parse();

Enum parsedColour = (Colour)Enum.Parse(typeof(Colour), "17");

Might be worth using to get a runtime error thrown, if you like.

CmdrTallen
  • 2,264
  • 4
  • 28
  • 50
  • 2
    As far as I can see, this doesn't actually work. It just sets parsedColour to 17, rather than throwing an exception. AN exception IS thrown if you pass some random string into it (eg. "asdf"). Or am I missing something? – Mike Chamberlain Nov 04 '10 at 00:21
  • After trying it, I can also concur that this does not work. This does not throw a runtime error. You can check it for yourself by making a quick unit test. – Gilles Apr 03 '14 at 12:47