What type of Exception should I throw when an unknown value is passed into a switch statement

Question

Edit 1

Updated to make the enum not an argument to the method...

Question

This type of problem comes up a lot with enums in switch statements. In the example code, the developer has accounted for all countries the program is currently using, but if another country is added to the Country enum, an exception should be thrown. My question is, what type of exception should be thrown?

Example Code:

enum Country
{
    UnitedStates, Mexico,
}

public string GetCallingCode(Guid countryId){
    var country = GetCountry(countryId);
    switch (country)
    {
        case Country.UnitedStates:
            return "1";
            break;
        case Country.Mexico:
            return "52";
            break;
        default:
            // What to throw here
        break;
    }
}

I've looked at

• NotImplemented, The exception that is thrown when a requested method or operation is not implemented.
• NotSupported There are methods that are not supported in the base class, with the expectation that these methods will be implemented in the derived classes instead. The derived class might implement only a subset of the methods from the base class, and throw NotSupportedException for the unsupported methods.
  For scenarios where it is sometimes possible for the object to perform the requested operation, and the object state determines whether the operation can be performed, see InvalidOperationException.
• InvalidOperation is used in cases when the failure to invoke a method is caused by reasons other than invalid arguments.

My guess is either NotImplemented or Invalid Operation. Which one should I use? Does someone have a better option (I know rolling your own is always an option)

Answer 1

I would go with ArgumentException, as the agrument is invalid.

EDIT: http://msdn.microsoft.com/en-us/library/system.argumentexception%28v=vs.71%29.aspx

There is also InvalidEnumArgumentException, which might more accurately describe the problem, however, I have not seen anyone use it before.

Answer 2

One option is to do almost a method contracts check in Debug mode. Throw in an extension method for nice looking form:

[Conditional("DEBUG")]
public static bool AssertIsValid(this System.Enum value)
{
    if (!System.Enum.IsDefined(value.GetType(), value))
        throw new EnumerationValueNotSupportedException(value.GetType(), value); //custom exception
}

I figured maybe only have it in debug mode so it passes your development/test environment and unit tests and in production there's no overhead (though that's up to you)

public string GetCallingCode(Guid countryId)
{
    var country = GetCountry(countryId);
    country.AssertIsValid(); //throws if the country is not defined
    
    switch (country)
    {
        case Country.UnitedStates:
            return "1";
        case Country.Mexico:
            return "52";
    }
}

I would suggest though that this is actually the responsibility of your GetCountry method. It should recognize that the countryId is not valid and throw an exception.

Regardless, this should really be caught by your unit tests too or somehow better handled. Wherever you convert a string/int into your enum should be handled by a singular method which in turn can check/throw (just as any Parse method should) and have a unit test that checks all valid numbers.

In general, I don't think the various ArgumentExceptions (and the like) are a good candidate because there are several conditions (non-argument) cases. I think if you move the checking code to a single spot, you may as well throw your own exception that accurately communicates to any developer listening.

EDIT: Considering the discussion, I think there are two particular cases here.

Case 1: Converting underlying type to an equivalent enum

If your methods take some sort of input data (string, int, Guid?), your code performing the conversion into the enum should validate that you have an actual enum that's usable. This is the case I posted above in my answer. In such cases, likely throwing your own exception or possibly InvalidEnumArgumentException.

This should be treated pretty much like any standard input validation. Somewhere in your system you are providing garbage-in so handle it as you would any other parsing mechanism.

var country = GetCountry(countryId);

switch (country)
{
    case Country.UnitedStates:
        return "1";
    case Country.Mexico:
        return "52";
}

private Country GetCountry(Guid countryId)
{
    //get country by ID
    
    if (couldNotFindCountry)
        throw new EnumerationValueNotSupportedException(.... // or InvalidEnumArgumentException

    return parsedCountry;
}

EDIT: of course, the compiler requires that your method throws/returns, so not so sure what you should do here. I guess that's up to you. If that actually happens, it probably is a bone-headed exception (case 2 below) since you passed your input validation yet did not update the switch/case to handle the new value, so maybe it should throw new BoneheadedException();

Case 2: Adding a new enumeration value which is not handled by your switch/case blocks in your code

If you are the owner of your code, this falls under the "Boneheaded" exceptions described by Eric Lippert in @NominSim's answer. Though this can actually not result in an exception at all while leaving the program in an exceptional/invalid state.

The best for this is likely any place where you perform switch/case (or of the like) runs against the enumeration, you should consider writing a unit test that automatically runs the method against all defined values of your enumeration. So if you are lazy or accidentally missed a block, your unit tests will warn you that you did not update a method to account for the change in your enumeration listing.

Finally, if your enum is coming from a 3rd party which you did not realize they updated the values, you should write a quick unit test that validates all your expected values. So if you wrote your program with checks for UnitedStates and Mexico, your unit test should just be a switch/case block for those values and throw an exception otherwise warning you when/if they end up adding Canada. When that test fails after updating the 3rd party library, you know what/where you have to make changes to be compatible.

So in this "Case 2", you should throw any old exception you want since it will be handled by your unit tests just so long as it accurately communicates to you or consumers of your unit tests just what exactly is missing.

---

In either case, I don't think the switch/case code should be caring too much about invalid input and not throwing exceptions there. They should either be thrown at design time (via unit tests) or thrown when validating/parsing input (and therefore throw appropriate "parsing/validation" exceptions)

---

EDIT: I stumbled upon a post from Eric Lippert discussing how the C# compiler detects if a method with a return value ever hits its "end point" without returning. The compiler is good at guaranteeing that the end point is unreachable sometimes, but in cases as yours above, we developers know it's unreachable (except in the circumstances noted above where BoneheadedExceptions come into play).

What wasn't discussed (at least that I saw) what you should do as a developer to resolve these cases. The compiler requires that you provide a return value or throw an exception even if you know it will never reach that code. Googling hasn't magically surfaced some exception to leverage in this case (though I couldn't figure out good search terms), and I'd rather throw an exception and be informed that my assumption that it cannot reach the end is incorrect rather than returning some value which may not inform me of the problem or result in unwanted behaviour. Maybe some UnexpectedCodePathFailedToReturnValueException of some sort would be most valid in this case. When I have some time, I'll do some more digging and maybe post a question on programmers to garner some discussion.

Answer 3

Of the exceptions you've listed, only InvalidOperationException fits your scenario. I would consider using either this, or either ArgumentException or the more specific ArgumentOutOfRangeException since your switch value is provided as an argument.

Or, as you say, roll your own.

EDIT: Based on your updated question, I would suggest InvalidOperationException if you want to use a framework exception. However, for this more generic case, I would definitely prefer to roll my own - you can't guarantee that InvalidOperationException won't be caught elsewhere in the callstack (possibly by the framework itself!), so using your own exception type is much more robust.