Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

What is the preferred method for handling unexpected enum values?

Suppose we have a method that accepts a value of an enumeration. After this method checks that the value is valid, it switches over the possible values. So the question is, what is the preferred method of handling unexpected values after the value range has been validated?

For example:

enum Mood { Happy, Sad }

public void PrintMood(Mood mood)
{
    if (!Enum.IsDefined(typeof(Mood), mood))
    {
        throw new ArgumentOutOfRangeException("mood");
    }

    switch (mood)
    {
        case Happy: Console.WriteLine("I am happy"); break;
        case Sad:   Console.WriteLine("I am sad"); break;
        default: // what should we do here?
    }

What is the preferred method of handling the default case?

  • Leave a comment like // can never happen
  • Debug.Fail() (or Debug.Assert(false))
  • throw new NotImplementedException() (or any other exception)
  • Some other way I haven't thought of
like image 729
Hosam Aly Avatar asked Mar 06 '09 18:03

Hosam Aly


4 Answers

I guess most of the above answers are valid, but I'm not sure any are correct.

The correct answer is, you very rarely switch in an OO language, it indicates you are doing your OO wrong. In this case, it's a perfect indication that your Enum class has problems.

You should just be calling Console.WriteLine(mood.moodMessage()), and defining moodMessage for each of the states.

If a new state is added--All Your Code Should Adapt Automatically, nothing should fail, throw an exception or need changes.

Edit: response to comment.

In your example, to be "Good OO" the functionality of the file mode would be controlled by the FileMode object. It could contain a delegate object with "open, read, write..." operations that are different for each FileMode, so File.open("name", FileMode.Create) could be implemented as (sorry about the lack of familiarity with the API):

open(String name, FileMode mode) {
    // May throw an exception if, for instance, mode is Open and file doesn't exist
    // May also create the file depending on Mode
    FileHandle fh = mode.getHandle(name);
    ... code to actually open fh here...
    // Let Truncate and append do their special handling
    mode.setPosition(fh);
}

This is much neater than trying to do it with switches... (by the way, the methods would be both package-private and possibly delegated to "Mode" classes)

When OO is done well, every single method looks like a few lines of really understandable, simple code--TOO simple. You always get the feeling that there is some big messy "Cheese Nucleus" holding together all the little nacho objects, but you can't ever find it--it's nachos all the way down...

like image 99
Bill K Avatar answered Oct 18 '22 22:10

Bill K


I prefer to throw new NotImplementedException("Unhandled Mood: " + mood). The point is that the enumeration may change in the future, and this method may not be updated accordingly. Throwing an exception seems to be the safest method.

I don't like the Debug.Fail() method, because the method may be part of a library, and the new values might not be tested in debug mode. Other applications using that library can face weird runtime behaviour in that case, while in the case of throwing an exception the error will be known immediately.

Note: NotImplementedException exists in commons.lang.

like image 27
Hosam Aly Avatar answered Oct 18 '22 21:10

Hosam Aly


In Java, the standard way is to throw an AssertionError, for two reasons:

  1. This ensures that even if asserts are disabled, an error is thrown.
  2. You're asserting that there are no other enum values, so AssertionError documents your assumptions better than NotImplementedException (which Java doesn't have anyway).
like image 7
Michael Myers Avatar answered Oct 18 '22 20:10

Michael Myers


My opinion is that since it is a programmer error you should either assert on it or throw a RuntimException (Java, or whatever the equivalent is for other languages). I have my own UnhandledEnumException that extends from RuntimeException that I use for this.

like image 3
TofuBeer Avatar answered Oct 18 '22 20:10

TofuBeer