I'm having a hard time describing this problem. Maybe that's why I'm having a hard time finding a good solution (the words just aren't cooperating). Let me explain via code:
// original code
enum Fruit
{
Apple,
Orange,
Banana,
}
...
Fruit fruit = acquireFruit();
if (fruit != Fruit.Orange && fruit != Fruit.Banana)
coreFruit();
else
pealFruit();
eatFruit();
Now pretend years of development go by with these three types. Different flavors of the above logic propagate throughout stored procedures, SSIS packages, windows apps, web apps, java apps, perl scripts and etc....
Finally:
// new code
enum Fruit
{
Apple,
Orange,
Banana,
Grape,
}
Most of the time, the "system" runs fine until Grapes are used. Then parts of the system act inappropriately, pealing and/or coring grapes when it's not needed or desired.
What kind of guidelines do you adhere to so these messes are avoided? My preference is for old code to throw an exception if it hasn't been refactored to consider new enumerations.
I've come up with a shot in the dark:
#1 Avoid "Not In Logic" such as this
// select fruit that needs to be cored
select Fruit from FruitBasket where FruitType not in(Orange, Banana)
#2 Use carefully constructed NotIn() methods when needed
internal static class EnumSafetyExtensions
{
/* By adding enums to these methods, you certify that 1.) ALL the logic inside this assembly is aware of the
* new enum value and 2.) ALL the new scenarios introduced with this new enum have been accounted for.
* Adding new enums to an IsNot() method without without carefully examining every reference will result in failure. */
public static bool IsNot(this SalesOrderType target, params SalesOrderType[] setb)
{
// SetA = known values - SetB
List<SalesOrderType> seta = new List<SalesOrderType>
{
SalesOrderType.Allowance,
SalesOrderType.NonAllowance,
SalesOrderType.CompanyOrder,
SalesOrderType.PersonalPurchase,
SalesOrderType.Allotment,
};
setb.ForEach(o => seta.Remove(o));
// if target is in SetA, target is not in SetB
if (seta.Contains(target))
return true;
// if target is in SetB, target is not not in SetB
if (setb.Contains(target))
return false;
// if the target is not in seta (the considered values minus the query values) and the target isn't in setb
// (the query values), then we've got a problem. We've encountered a value that this assembly does not support.
throw new InvalidOperationException("Unconsidered Value detected: SalesOrderType." + target.ToString());
}
}
Now, I can safely, use code like this:
bool needsCoring = fruit.IsNot(Fruit.Orange, Fruit.Banana);
If this code gets propagated throughout the system, exceptions will be thrown when the Grape comes rolling into town (qa will catch 'em all).
That's the plan anyway. The problem seems like it should be very common, but I can't seem to find anything on google (probably my own fault).
How are you all handling this?
UPDATE:
I feel the answer to this problem is create a "catch everything else" mechanism that halts processing and alerts testers and developers to that fact that the new enumeration needs consideration. "switch ... default" is great if you have it.
If C# didn't have switch ... default, we might right the above code like this:
Fruit fruit = acquireFruit();
if (fruit != Fruit.Orange && fruit != Fruit.Banana)
coreFruit();
else if(fruit == Fruit.Apple)
pealFruit();
else
throw new NotSupportedException("Unknown Fruit:" + fruit)
eatFruit();
DISCLAIMER:
You really shouldn't use any of the above pseudo code. It may(?) compile or even work, but it's horrible code, really. I saw a lot of nice solutions in this thread if you're looking for an OOP-based approach. A good solution, of course, places all the switching and checking in a centralized method (a factory method is what strikes me). Peer code review on top of that will also be required.
An enumeration, or Enum , is a symbolic name for a set of values. Enumerations are treated as data types, and you can use them to create sets of constants for use with variables and properties.
An enumeration is a user-defined data type that consists of integral constants. To define an enumeration, keyword enum is used. enum season { spring, summer, autumn, winter }; Here, the name of the enumeration is season .
An enum is defined using the enum keyword, directly inside a namespace, class, or structure. All the constant names can be declared inside the curly brackets and separated by a comma. The following defines an enum for the weekdays. Above, the WeekDays enum declares members in each line separated by a comma.
If I understood your question correctly, the most common practice is to throw an NotSupportedException
or NotImplementedException
.
switch (fruit.Kind) {
case Fruit.Apple:
Bite(fruit);
break;
case Fruit.Banana:
FeedToMonkey(fruit);
break;
default: throw new NotSupportedException("Unknown fruit.");
}
As for adding new enum values which would break existing if-not-is logic, I believe using enum is a poor choice in this case. Your items clearly have a distinctively different behavior, they're not like e.g. colors. Perhaps it is best to make the options responsible for deciding how they should be treated. Then you should replace enums with polymorphism.
I would use types, not enums, for the data structures... E.G. Create an interface IFruit
that has the following:
interface IFruit
{
bool NeedsCoring();
void GetEaten(Person by);
// etc.
}
And then I would just call the methods already there for determining whether it needs to be cored or whatnot.
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