views:

641

answers:

8

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.

+2  A: 

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
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
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
+6  A: 
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.

        }
    }
}
Greco
A: 

I think it depends on what you want your code to do if a different option is provided. It also makes a difference if this is internal code or a public API. In some cases you may want to do nothing, in other cases you may want to throw an exception.

Finally, don't forget that you can cast any old int to your enum, so the input really could be anything.

Jon B
+1  A: 

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
+8  A: 

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
+13  A: 

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
That is the crux of it!
kronoz
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
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
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
A: 

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.

Timothy Carter
+11  A: 

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
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
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