-----------------------------------------------------------------
Hey guys!
Lets consider the following C# code (the issue is applicable for many languages as well):
public enum Frequency{
Monthly = 1
}
public int Method(Frequency value){switch (value){case Frequency.Monthly:return 1 + 2 + 3; // Some logicdefault:throw new Exception($"{value} can not be handled!");}}
Programming language does not matter here, because we are going to argue about a valid exception in default clause. What are your variants? Below are standard handling for similar situation and I believe we should always follow it:
- ArgumentOutOfRangeException should be thrown if you pass invalid enum value, like following: Method((Frequency) -1) - this is allowed by C#, but can be considered like a hack.
- NotSupportedException should be thrown if you pass a valid enum value (e.t. enum has also additional value Frequency.Daily and we pass it into this method) into a method which does not know how to process it. There is no handling for this value right now, so we must inform calling code that the method does not support/provide an implementation for the passed value.
///////////////////////////////////////////////////////////////////////////////
What we have in our code above:
- Enum has the only value which is handled by switch operator. So right now ArgumentOutOfRangeException is not bad.
///////////////////////////////////////////////////////////////////////////////
And do you know what? Sometimes we face with such code (keeping in mind that enum still has the only value):
public int Method(Frequencyvalue){switch (value){case Frequency.Monthly:return 1 + 2 + 3; // Some logicdefault:throw new NotSupportedException($"{value} is not supported!");}}
NotSupportedException is thrown in default clause only for invalid values, because we have the only value in our enum which is handled by switch operator.
Invalid values are out of range and this is why from my point of view NotSupportedException does not explain a real reason of this exceptional case.
But why we have NotSupportedException here?
Some developers who vote for this approach explain it in a simple way: "Right now this method supports all existing values and if our enum will be updated with new value in the future, the method will throw a valid exception w/o method updating."
Hmmm... Quite rational point from the first sight, similar on Open/Close principle, but is it true? Lets take a deep look: there is a time period when this code has wrong semantic with NotSupportedException for out of range values (until a new enum value is introduced). Are you OK with this?
Also they say "We avoid redundant method's change in the future". Do you believe them? Which exception do we expect in this case Method((Frequency) -1) (if we allow such casting)? I would expect ArgumentOutOfRangeException. But NotSupportedException will be thrown even in this case! To distinguish these cases we must update our method or it will have a wrong semantic AGAIN...
Also lets think from another side: how often do we update an enum and leave such switch-methods w/o update? I would say this is an exceptional case in normal life: usually with introducing a new value for enum, we introduce a new branch of logic. As a result you should review all code places where the updated enum is used and update them as well.
Conslusion
You should not think "too much" about future of the code, just keep in mind that your code must be clear and clean here and now!
Think out of the box! Make your right switch! :)
P.S. Did you hear about Enum.IsDefined method? It can be very useful to determine when ArgumentOutOfRangeException can be thrown ;)
