Suppose we have a method that accepts a value of an enumeration. After this method checks that the value is valid, it switch
es 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?
// can never happen
Debug.Fail()
(or Debug.Assert(false)
)throw new NotImplementedException()
(or any other exception)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...
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
.
In Java, the standard way is to throw an AssertionError
, for two reasons:
asserts
are disabled, an error is thrown.AssertionError
documents your assumptions better than NotImplementedException
(which Java doesn't have anyway).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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With