41

This has been a pet peeve of mine since I started using .NET but I was curious in case I was missing something. My code snippet won't compile (please forgive the forced nature of the sample) because (according to the compiler) a return statement is missing:

public enum Decision { Yes, No}

    public class Test
    {
        public string GetDecision(Decision decision)
        {
            switch (decision)
            {
                case Decision.Yes:
                    return "Yes, that's my decision";
                case Decision.No:
                    return "No, that's my decision";

            }
        }
    }

Now I know I can simply place a default statement to get rid of the complier warning, but to my mind not only is that redundant code, its dangerous code. If the enumeration was in another file and another developer comes along and adds Maybe to my enumeration it would be handled by my default clause which knows nothing about Maybes and there's a really good chance we're introducing a logic error.

Whereas, if the compiler let me use my code above, it could then identify that we have a problem as my case statement would no longer cover all the values from my enumeration. Sure sounds a lot safer to me.

This is just so fundamentally wrong to me that I want to know if its just something I'm missing, or do we just have to be very careful when we use enumerations in switch statements?

EDIT: I know I can raise exceptions in the default or add a return outside of the switch, but this are still fundamentally hacks to get round a compiler error that shouldn't be an error.

With regards an enum really just being an int, that's one of .NET's dirty little secrets which is quite embarassing really. Let me declare an enumeration with a finite number of possibilities please and give me a compilation for:

Decision fred = (Decision)123;

and then throw an exception if somebody tries something like:

int foo = 123;
Decision fred = (Decision)foo;

EDIT 2:

A few people have made comments about what happens when the enum is in a different assembly and how this would result in problems. My point is that this is the behaviour I think should happen. If I change a method signature this will lead to issues, my premis is that changing an enumeration should be this same. I get the impression that a lot of people don't think I understand about enums in .NET. I do I just think that the behaviour is wrong, and I'd hoped that someone might have known about some very obscure feature that would have altered my opinion about .NET enums.

Mark
  • 2,392
  • 4
  • 21
  • 42
  • 10
    I think you mean "public enum Decision { Yes, No, FileNotFound }" – Juliet Jul 08 '09 at 15:07
  • 1
    nice one Juliet... but you should probably make it clear that it's a joke, I'm not sure this is obvious for everybody ;) – Thomas Levesque Jul 08 '09 at 15:10
  • In what way is this "a compiler error that shouldn't be an error"? Your code is wrong, and the fact that you happen to be switching on an `enum` has nothing to do with the compiler error. Try switching on any other type without including a `default` section and you'll see the same error. (Even if you use a `bool`, which can only ever be in one of two states.) – LukeH Jul 08 '09 at 16:34
  • I'm not seeing the problem here. Since enums *can* change over time, it only makes sense to require that you provide a default case. If it didn't require one, then I could see that being the case of giving you the rope to hang yourself. For example, if your enum lives in a different assembly and that assembly is changed without yours being recompiled, what would happen? Your program would crash at runtime. It makes perfect sense to require a dfault case since enums are modifiable at any point. Bools on the other hand... that one doesn't make sense. – jasonh Jul 08 '09 at 17:26
  • 2
    @jasonh working on your basis that enums can change so can methods, does that mean that all classes should have a default method to fall back on in case .NET can't find the method signature you requested? – Mark Jul 08 '09 at 18:01
  • 1
    @Luke the compiler is telling me that there is a return statement missing because due to the .NET implimentation of an enum it can find a code path that doesn't find have an return value. That is only true because .NET's enums aren't true enumerations. If they were there are only two possible routes through the routine. – Mark Jul 08 '09 at 18:05
  • 2
    @Mark: This isn't due to the .NET implementation of `enum`, it's just how the `switch` statement works: You'll get the same error if you use a `bool` and cover the `true` and `false` paths but omit the `default`; or if you use a `byte` and cover all 256 possible paths without a `default`; or use a short and cover all 65536 paths without a `default` etc etc. – LukeH Jul 08 '09 at 22:48
  • The compiler makes sense. For an unspecified case, you would land after the switch, with no remaining return statement. The way I see it, you have two very valid choices: (A) add a default that throws a NotImplementedException, which makes sense to protect against potential new enum values *and other integral values that your enum does not define, but may certainly be passed to this method*, or (B) add a return statement with a comment that indicates why it could be *valid* to get here and consciously do nothing. – Timo Mar 04 '16 at 13:52

10 Answers10

45

Heck, the situation is far worse than just dealing with enums. We don't even do this for bools!

public class Test {        
  public string GetDecision(bool decision) {
    switch (decision) {
       case true: return "Yes, that's my decision";                
       case false: return "No, that's my decision"; 
    }
  }
}

Produces the same error.

Even if you solved all the problems with enums being able to take on any value, you'd still have this issue. The flow analysis rules of the language simply do not consider switches without defaults to be "exhaustive" of all possible code paths, even when you and I know they are.

I would like very much to fix that, but frankly, we have many higher priorities than fixing this silly little issue, so we've never gotten around to it.

Eric Lippert
  • 647,829
  • 179
  • 1,238
  • 2,067
  • 4
    Thanks Eric for being able to see through my ramblings to the heart of the issue. However, don't you see that in the case of the enumeration is can actually lead to problems, by forcing a redundant default case that me turn into a dangerous one. – Mark Jul 08 '09 at 18:10
  • 10
    Yep. The fact that enums are nothing more than fancy ints, and the fact that enums can change from version to version, make them dangerous to use. That is unfortunate; the next time you design a type system from scratch, you'll know not to repeat this mistake. – Eric Lippert Jul 08 '09 at 20:08
  • This one seems to be fix now :) – Wouter Jul 21 '22 at 12:12
23

Throw an exception in the default clause:

default:
    throw new ArgumentOutOfRangeException("decision");

This ensures that all possible paths are covered, while avoiding the logic errors resulting from adding a new value.

Bryan Watts
  • 44,911
  • 16
  • 83
  • 88
20

That's because the value of decision could actually be a value that is not part of the enumeration, for instance :

string s = GetDecision((Decision)42);

This kind of thing is not prevented by the compiler or the CLR. The value could also be a combination of enum values :

string s = GetDecision(Decision.Yes | Decision.No);

(even if the enum doesn't have the Flags attribute)

Because of that, you should always put a default case in you switch, since you can't check all possible values explicitly

Thomas Levesque
  • 286,951
  • 70
  • 623
  • 758
  • I appreciate that you can do those things, but to me they're just cases of .NET giving you the rope to hang yourself. – Mark Jul 08 '09 at 15:24
  • Well, actually that makes sense for enums with the `Flags` attribute... this attribute means that you can combine the values, which often results in values that are not explicitly part of the enum, but are valid nevertheless. However I would like the compiler to allow this only for `Flags` enums... – Thomas Levesque Jul 08 '09 at 16:19
  • 1
    Actually, it's giving you enough rope to efficiently interoperate with existing COM and WIN32 apis that define enumerated integral types for their flags. But yes, you have to be careful with that rope. – Eric Lippert Jul 08 '09 at 16:21
9
public enum Decision { Yes, No}

public class Test
{
    public string GetDecision(Decision decision)
    {
        switch (decision)
        {
            case Decision.Yes:
                return "Yes, that's my decision";
            case Decision.No:
                return "No, that's my decision";
            default: throw new Exception(); // raise exception here.

        }
    }
}
bdukes
  • 152,002
  • 23
  • 148
  • 175
Dimi Takis
  • 4,924
  • 3
  • 29
  • 41
3

The default is there to protect you. Throw an exception from the default and if anyone adds an extra enum, you're covered with something to flag it.

Joel Goodwin
  • 5,026
  • 27
  • 30
  • I know I can do that but its still introducing a runtime exception that could be avoided. .NET forces you to use breaks rather than allowing fall-throughs to avoid logic errors forcing the use of a default is just asking for trouble. – Mark Jul 08 '09 at 15:11
  • 1
    It can't be caught at compile-time because of what Thomas L said, so the default is there to make sure it is caught. – Joel Goodwin Jul 08 '09 at 15:19
2

I realize that this is a thread resurrection...

I personally feel that the way that switch works is correct, and it operates how I would logically want it to.

I'm surprised to hear such complaining about the default label.

If you only have a strict set of enums or values that you are testing, you don't need all of the exception handling lines, or a return outside of the switch, etc.

Just put the default label over one of the other labels, perhaps the label that would be the most common response. In your example case it probably doesn't matter which one. Short, sweet, and it fulfills your needs to rid of the compiler warning:

switch (decision)
{
    default:
    case Decision.Yes:
        return "Yes, that's my decision";
    case Decision.No:
        return "No, that's my decision";
}

If you don't want the default to be Yes, put the default label above the No label.

deegee
  • 1,553
  • 14
  • 13
  • 1
    Again, I realise that this is a thread resurrection - the irony is not lost on me... The OP's point is that if you were to add a `Decision.Maybe` to your enum then your suggestion wouldn't work. None of the current code paths apply for `Maybe` so your code will be hiding a bug. If you were allowed to omit the default the compiler could say "Hang on! You haven't returned anything if the decision was maybe!" and at compile time you would have the potential bug flagged to you. I suspect the OP is aware that you can put default onto an existing branch, its just not what they want to have to do. – Chris Feb 22 '18 at 09:26
2

For the sake of sharing a quirky idea if nothing else, here goes:

You can always implement your own strong enums

...and since the introduction of the nameof operator you can also use them in switch-cases. (Not that you couldn't technically do so previously, but it was difficult to make such code readable and refactor friendly.)

public struct MyEnum : IEquatable<MyEnum>
{
    private readonly string name;
    private MyEnum(string name) { name = name; }

    public string Name
    {
        // ensure observable pureness and true valuetype behavior of our enum
        get { return name ?? nameof(Bork); } // <- by choosing a default here.
    }

    // our enum values:
    public static readonly MyEnum Bork;
    public static readonly MyEnum Foo;
    public static readonly MyEnum Bar;
    public static readonly MyEnum Bas;

    // automatic initialization:
    static MyEnum()
    {
        FieldInfo[] values = typeof(MyEnum).GetFields(BindingFlags.Static | BindingFlags.Public);
        foreach (var value in values)
            value.SetValue(null, new MyEnum(value.Name));
    }

    /* don't forget these: */
    public override bool Equals(object obj)
    {
        return obj is MyEnum && Equals((MyEnum)obj);
    }
    public override int GetHashCode()
    {
        return Name.GetHashCode();
    }
    public override string ToString()
    {
        return Name.ToString();
    }
    public bool Equals(MyEnum other)
    {
        return Name.Equals(other.Name);
    }
    public static bool operator ==(MyEnum left, MyEnum right)
    {
        return left.Equals(right);
    }
    public static bool operator !=(MyEnum left, MyEnum right)
    {
        return !left.Equals(right);
    }
}

and use it thusly:

public int Example(MyEnum value)
{
    switch(value.Name)
    {
        default: //case nameof(MyEnum.Bork):
            return 0;
        case nameof(MyEnum.Foo):
            return 1;
        case nameof(MyEnum.Bar):
            return 2;
        case nameof(MyEnum.Bas):
            return 3;
    }
}

and you would of course call that method like so:
int test = Example(MyEnum.Bar); // returns 2

That we can now easily get the Name is basically just a bonus, and yeah some readers might point out that this is basically a Java enum without the null-case (since it's not a class). And just like in Java you can add whatever extra data and or properties you desire to it, e.g. an ordinal value.

Readability: Check!
Intellisense: Check!
Refactorability: Check!
Is a ValueType: Check!
True enumeration: Check!
...
Is it performant? Compared to native enums; no.
Should you use this? Hmmm....

How important is it for you to have true enumerations so you can getting rid of enum runtime checks and their accompanying exceptions?
I don't know. Can't really answer that for you dear reader; to each their own.

...Actually, as I wrote this I realized it would probably be cleaner to let the struct "wrap" a normal enum. (The static struct fields and the corresponding normal enum mirroring each other with the help of similar reflection as above.) Just never use the normal enum as a parameter and you're good.

UPDATE :

Yepp, spent the night testing out my ideas, and I was right: I now have near perfect java-style enums in c#. Usage is clean and performance is improved. Best of all: all the nasty shit is encapsulated in the base-class, your own concrete implementation can be as clean as this:

// example java-style enum:
public sealed class Example : Enumeration<Example, Example.Const>, IEnumerationMarker
{
    private Example () {}

    /// <summary> Declare your enum constants here - and document them. </summary>
    public static readonly Example Foo = new Example ();
    public static readonly Example Bar = new Example ();
    public static readonly Example Bas = new Example ();

    // mirror your declaration here:
    public enum Const
    {
        Foo,
        Bar,
        Bas,
    }
}

This is what you can do:

  • You can add any private fields you want.
  • You can add any public non-static fields you want.
  • You can add any properties and methods you want.
  • You can design your constructors however you wish, because:
  • You can forget base constructor hassle. Base constructor is parameter-less!

This is what you must do:

  1. Your enum must be a sealed class.
  2. All your constructors must be private.
  3. Your enum must inherit directly from Enumeration<T, U> and inherit the empty IEnumerationMarker interface.
  4. The first generic type parameter to Enumeration<T, U> must be your enum class.
  5. For each public static field there must exist an identically named value in the System.Enum (that you specified as the second generic type parameter to Enumeration<T, U>).
  6. All your public static fields must be readonly and of your enum type.
  7. All your public static fields must be assigned a unique non-null value during type initialization.

At the moment every invariant above is asserted at type initialization. Might try to tweak it later to see if some of it can be detected at compile-time.

Requirements Rationale:

  1. Your enum must be sealed because if it isn't then other invariants become a lot more complicated for no obvious benefit.
  2. Allowing public constructors makes no sense. It's an enumeration type, which is basically a singleton type but with a fixed set of instances instead of just one.
  3. Same reason as the first one. The reflection and some of the other invariant and constraint checking just becomes messy if it isn't.
  4. We need this generic type parameter so the type-system can be used to uniquely store our enum data with performant compile/Jit time bindings. No hash-tables or other slow mechanisms are used! In theory it can be removed, but I don't think it's worth the added cost in complexity and performance to do so.
  5. This one should be quite obvious. We need these constants for making elegant switch-statements. Of course I could make a second enum-type that doesn't have them; you would still be able to switch using the nameof method shown earlier. It just would not be as performant. I'm still contemplating if a should relax this requirement or not. I'll experiment on it...
  6. Your enum constants must be public and static for obvious reasons, and readonly fields because a) having readonly enumeration instances means all equality checks simplifies to reference equality; b) properties are more flexible and verbose, and frankly both of those are undesirable properties for an enumeration implementation. Lastly all public static fields must be of your enum-type simply because; a) it keeps your enumeration type cleaner with less clutter; b) makes the reflection simpler; and c) you are free to do whatever with properties anyway so it's a very soft restriction.
  7. This is because I'm trying hard to keep "nasty reflection magic" to a minimum. I don't want my enum implementation to require full-trust execution. That would severely limit its usefulness. More precisely, calling private constructors or writing to readonly fields can throw security exceptions in a low-trust environment. Thus your enum must instantiate your enum constants at initialization time itself - then we can populate the (internal) base-class data of those instances "cleanly".

So anyway, how can you use these java-style enums?

Well I implemented this stuff for now:

int ordinal = Example.Bar.Ordinal; // will be in range: [0, Count-1]
string name = Example.Bas.Name; // "Bas"
int count = Enumeration.Count<Example>(); // 3
var value = Example.Foo.Value; // <-- Example.Const.Foo

Example[] values;
Enumeration.Values(out values);

foreach (var value in Enumeration.Values<Example>())
    Console.WriteLine(value); // "Foo", "Bar", "Bas"

public int Switching(Example value)
{
    if (value == null)
        return -1;

    // since we are switching on a System.Enum tabbing to autocomplete all cases works!
    switch (value.Value)
    {
        case Example.Const.Foo:
            return 12345;
        case Example.Const.Bar:
            return value.GetHasCode();
        case Example.Const.Bas:
            return value.Ordinal * 42;
        default:
            return 0;
    }
}

The abstract Enumeration class will also implement the IEquatable<Example> interface for us, including == and != operators that will work on Example instances.

Aside from the reflection needed during type initialization everything is clean and performant. Will probably move on to implement the specialized collections java has for enums.

So where is this code then?

I want to see if I can clean it up a bit more before I release it, but it will probably be up on a dev branch on GitHub by the end of the week - unless I find other crazy projects to work on! ^_^

Now up on GitHub
See Enumeration.cs and Enumeration_T2.cs.
They are currently part of the dev branch of a very much wip library I'm working on.
(Nothing is "releasable" yet and subject to breaking changes at any moment.)
...For now the rest of the library is mostly a shit ton of boilerplate to extend all array methods to multi-rank arrays, make multi-rank arrays usable with Linq, and performant ReadOnlyArray wrappers (immutable structs) for exposing (private) arrays in a safe way without the cringy need to create copies all the time.

Everything* except the very latest dev commits is fully documented and IntelliSense friendly.
(*The java enum types are still wip and will be properly documented once I've finalized their design.)

AnorZaken
  • 1,916
  • 1
  • 22
  • 34
  • Someone could still update the MyEnum and any switch without a default exception will cause logical errorrs. – Wouter Jul 21 '22 at 12:01
  • @Wouter Of course, that's true for standard Enums too. A good programmer never changes an enum. And a smart programmer never leaves out the default case. – AnorZaken Jul 21 '22 at 12:51
  • 1
    True see also the don'ts from: https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/enum now try convincing the non-developers... they usually don't care as do most developers... so personally I throw an exception so at least I know someone didn't respect the guidelines. – Wouter Jul 21 '22 at 14:20
  • @Wouter Smart enums are _definitely_ not the solution for every problem, but with smart enums you can potentially encapsulate some of the responsibility into the enum type itself. Say for example you use the enum to decide how a text should be formatted, then each enum value could either have a property containing a format string, or even have a method that takes a string and formats it. Now if you add a new enum value, it either wont compile (format string into enum value ctor) or it is at least very unlikely to forget with the format method residing inside the smart-enum class. – AnorZaken Jul 21 '22 at 17:26
  • @Wouter For a simpler benefit: C# enums can have _any_ value regardless of what constants are defined. So even if the enum Decision only has the values Yes and No, anyone can still send in the value 2613 or whatever... This is not possible with smart enums. So if you are writing a library, and you define a smart enum, and you have methods with smart enum parameters, then you can be *100%* sure the user cannot send in some random nonsense, so you don't need to deal with that possibility logically, you don't need to document it, and you don't need to test it - because it's literally impossible. – AnorZaken Jul 21 '22 at 17:40
  • The smart enum isn't a valuetype and can be null so testing is still needed. I don't think value types have responsibilities. For libraries I just use Enum.IsDefined(...)) throw ArgumentOutOfRangeException(...) as I do with all arguments. Now there are other concerns for enums. How about storing a value (the integer the name or maybe link it with a table what if the enum values in code or in the table change. How about selecting a value do we need an "Unknown"(e.g. DayOfWeek.Unknown but that isn't a day right?) – Wouter Jul 21 '22 at 19:00
  • @Wouter That's only if you allow null in the first place. As for responsibilities, who said it's a value type - what I mean is there is nothing inherently value-type about objects of a Set. We are composing with smart objects here. As for the rest of what you are talking about concerns mappings between dtos / models / whatever, and obviously if you can store it in a DB for example then the enum class would have ToDbValue, FromDbValue methods ...we are basically describing serialization here, which is something very normal to implement inside the type being serialized. – AnorZaken Jul 21 '22 at 20:38
  • Even simpler the enum probably has some property, and we store the value of that property in the DB. The good thing is that if your method cannot throw, because bad values cannot be supplied, then the caller doesn't need code for handling any specific exception related to that either. – AnorZaken Jul 21 '22 at 20:40
  • To clarify, my argument is that everything you can do with enums, the smart enum can do better or at worst equivalent, and it can also do things the regular enum cannot. This is a statement that doesn't require any proof since it simply follows from the fact that smart enum allow you to fallback to regular enum values. Thus in the worst case you can always use the smart enum value as a regular enum value if you find a situation where that makes more sense. It's not like I'm saying smart enums is a silver bullet - but they should be better or equivalent in all situations. – AnorZaken Jul 21 '22 at 20:58
1

I always think of that default as the fall through/exception.

So here it would not be maybe but instead would be "Invalid Decision, contact support".

I don't see how it would fall through to that but that would be the catchall/exception case.

Maestro1024
  • 3,173
  • 8
  • 35
  • 52
0

In addition to the case of, you can cast any int to your enum and have an enum you aren't handling. There is also the case where, if the enum is in an external .dll, and that .dll is updated, it does not break your code if an additional option is added to the enum (like Yes, No, Maybe). So, to handle those future changes you need the default case as well. There is no way to guarantee at compile time that you know every value that enum will have for it's life.

Carl Manaster
  • 39,912
  • 17
  • 102
  • 155
Timothy Carter
  • 15,459
  • 7
  • 44
  • 62
0

Instead of complaining about how the switch statement works, avoid it entirely by using an extension method on the enum as explained here and here.

The nice thing about this approach is, you don't get into a situation and forgetting to update your GetDecision switch statement when adding a new enum value because it's all together in the same place - in the enum declaration.

The efficiency of this approach is not known to me and, really, not even a consideration at this point. That's right, I just don't care because it's so much easier for me that I figure, "Phhhtt screw the computer that's what it's there for - to work hard." (I may regret that attitude when Skynet takes over.)

If I ever need to get from one of those attribute values back to the enum value, I can simply build a reverse dictionary of and fill it with a single line of code.

(I usually do a 'not set' as the first enum element because, as the OP notes, a C# enum is really an int so an uninitialized variable is going to be zero or the first enum value)

public enum Decision
{
  [DisplayText("Not Set")]
  NotSet,
  [DisplayText("Yes, that's my decision")]
  Yes,
  [DisplayText("No, that's my decision")]
  No
}

public static class XtensionMethods
{
  public static string ToDisplayText(this Enum Value)
  {
    try
    {
      Type type = Value.GetType();
      MemberInfo[] memInfo =
        type.GetMember(Value.ToString());

      if (memInfo != null && memInfo.Length > 0)
      {
        object[] attrs = memInfo[0]
          .GetCustomAttributes(typeof(DisplayText), false);
        if (attrs != null && attrs.Length > 0)
          return ((DisplayText)attrs[0]).DisplayedText;
      }
    }
    catch (Exception ex)
    {
      throw new Exception(
        "Error in XtensionMethods.ToDisplayText(Enum):\r\n" + ex.Message);
    }
    return Value.ToString();
  }

  [System.AttributeUsage(System.AttributeTargets.Field)]
  public class DisplayText : System.Attribute
  {
    public string DisplayedText;

    public DisplayText(string displayText)
    {
      DisplayedText = displayText;
    }
  }
}

Use inline like:

myEnum.ToDisplayText();

Or wrapped in a function, if you feel like it:

public string GetDecision(Decision decision)
{
  return decision.ToDisplayText();
}
B H
  • 1,730
  • 18
  • 24