How do you write code whose logic is protected against future additional enumerations?

Question

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.

Answer 1

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.

Answer 2

While I don't have a ton of C# experience, were this Java, what I would instead do is create an interface IPeelable and another ICoreable, and have the fruit classes implement those. Then, instead of avoiding not logic, you could simply check to see if the fruit you've gotten implements either of the interfaces -- in this way, you can add future fruits which implement both peelable and coreable, like the muskmelon.

Answer 3

You cannot hold two pieces of data in one data store. You need to store two pieces of data, and therefore an enum is the wrong data type for this. These should be instances of a Fruit class.

Answer 4

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 make use inheritance instead of enums.

Answer 5

Generally speaking, conditional logic based on types (enumerated or regular) are going to break like this. When a new type is added, the compiler won't force you to update the switch.

For this example, instead of placing the majority of my logic in a switch, I would use polymorphism. I would pass the enum to a factory method/class, get back a fruit interface (or base fruit type), and execute virtual methods on the interface (/base type). I have to use a switch in the factory, because there is no other way to implement this that isn't logically identical. But because I return an abstract base class, I only have to do this switch in one place. I make sure to wash my hands afterwards :)

using System;

enum FruitType
{
    Apple,
    Banana,
    Pineapple,
}

interface IFruit
{
    void Prepare();
    void Eat();
}

class Apple : IFruit
{
    public void Prepare()
    {
        // Wash
    }

    public void Eat()
    {
        // Chew and swallow
    }
}

class Banana : IFruit
{
    public void Prepare()
    {
        // Peel
    }

    public void Eat()
    {
        // Feed to your dog?
    }
}

class Pineapple : IFruit
{
    public void Prepare()
    {
        // Core
        // Peel
    }

    public void Eat()
    {
        // Cut up first
        // Then, apply directly to the forehead!
    }
}

class FruitFactory
{
    public IFruit GetInstance(FruitType fruitType)
    {
        switch (fruitType)
        {
            case FruitType.Apple:
                return new Apple();
            case FruitType.Banana:
                return new Banana();
            case FruitType.Pineapple:
                return new Pineapple();
            default:
                throw new NotImplementedException(
                    string.Format("Fruit type not yet supported: {0}"
                        , fruitType
                    ));
        }
    }
}

class Program
{
    static FruitType AcquireFruit()
    {
        // Todo: Read this from somewhere.  A database or config file?
        return FruitType.Pineapple;
    }

    static void Main(string[] args)
    {
        var fruitFactory = new FruitFactory();
        FruitType fruitType = AcquireFruit();
        IFruit fruit = fruitFactory.GetInstance(fruitType);
        fruit.Prepare();
        fruit.Eat();
    }
}

The reason I went for a Prepare design, rather than a Core/Peel/Deseed/Dehusk/Chill/Cut design is that each fruit will need different preparation. With the design that separates preparation methods, you'll have to maintain all calling code (and possibly each implementation) each time you add a new class with different requirements. With the design that hides the specific preparation details, you can maintain each class separately, and adding new fruits doesn't break existing ones.

See this article for why my design is preferrable:

C++ FAQ Lite - Virtual Functions

Answer 6

This is a valid concern, and comes up most often when you're writing code against enums that are defined outside of your control, and which may evolve independently of your code. I'm referring to enums defined by the operating system or your execution environment, such as .NET itself. .NET (policy) explicitly allows enums to be extended across revisions of referenced assemblies, and this is not considered a breaking change.

One the the most common approaches is simply to write your code to throw an exception if it ever receives an enum it isn't prepared for. This is a simple solution, but it may be overkill depending on what you're using the enums for.

If you're using the enum to decide which of a fixed set of operations to perform, you don't have much choice but to throw if you encounter an unexpected enum.

However, if you're using the enum to decide if you should perform additional specialized operations on top of default behavior, then it's not necessarily the case that unexpected enums are the end of the world.

It's a judgment call that only you can make in the context of what your doing and in the context of what the enum actually represents. If you can write default behavior that you have high confidence should be acceptable for all enum cases, then maybe you can decide to accept future enum extensions without having to rewrite your code. This judgment call also requires evaluating your level of faith that the owner of the enum will extend the enum appropriately by not throwing completely unrelated stuff into the enum that requires completely different handling.

Answer 7

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.

Seems to me like the problem is an introduction of a new data type. You might want to consider modeling your classes using a kind of visitor pattern, particularly since this pattern is intended for related objects with a fixed number of well-defined data types:

public abstract class Fruit {
    public abstract T Match(Func<Apple, T> f, Func<Banana, T> g, Func<Grape, T> h);

    public class Apple {
        // apple properties
        public override T Match(Func<Apple, T> f, Func<Banana, T> g, Func<Grape, T> h) {
            return f(this);
        }
    }
    public class Banana {
        // banana properties
        public override T Match(Func<Apple, T> f, Func<Banana, T> g, Func<Grape, T> h) {
            return g(this);
        }
    }
    public class Grape {
        // grape properties
        public override T Match(Func<Apple, T> f, Func<Banana, T> g, Func<Grape, T> h) {
            return h(this);
        }
    }
}

Usage:

public void EatFruit(Fruit fruit, Person p)
{
    // prepare fruit
    fruit.Match(
        apple => apple.Core(),
        banana => banana.Peel(),
        grape => { } // no steps required to prepare
        );

    p.Eat(fruit);
}

public FruitBasket PartitionFruits(List<Fruit> fruits)
{
    List<Apple> apples = new List<Apple>();
    List<Banana> bananas = new List<Banana>();
    List<Grape> grapes = new List<Grape>();

    foreach(Fruit fruit in fruits)
    {
        // partition by type, 100% type-safe on compile,
        // does not require a run-time type test
        fruit.Match(
            apple => apples.Add(apple),
            banana => bananas.Add(banana),
            grape => grapes.Add(grape));
    }

    return new FruitBasket(apples, bananas, grapes);
}

This style is advatageous for three reasons:

• Future proofing: Lets say I add a Pineapple type and add it to my Match method: Match(..., Func<Pineapple, T> k);. Now I have a bunch of compilation errors, because all current usages of Match pass in 3 params, but we expect 4. The code doesn't compile until fix all usages of Match to handle your new type -- this makes it impossible to introduce a new type at the risk of not being handled in your code.

• Type safety: The Match statement gives you access to specific properties of sub-types without a runtime type-test.

• Refactorable: If you don't like delegates as shown above, or you have several dozen types and don't want to handle them all, its perfectly easy to wrap those delegates by a FruitVisitor class, so each subtype passes itself to the appropriate method it the FruitVisitor.

Answer 8

You're asking the wrong questions. You're looking for coding work arounds for what is inadequate design. You want to add to your enumeration with minimum hassle.

I like your idea for using an enum for fruit types, but I'd have that as a field in a Fruit class.

I would use a class rather than an interface. You want to capture the concept of a fruit. It's a concrete thing. OTOH an interface would be good if you wanted to add the behavior or "quality" of being "fruitable". You want to have lots of different types of "Fruit" (a class), you are not adding "fruit-ability" (an interface) to an otherwise non-fruit thingy.

Rather than having a base "Fruit" class and sub-classing for every kind of fruit, just have an instance variable for the type - and make it an enumeration. Otherwise the number of sub-classes can get out of hand. Now every kind is a "Fruit". The "type" field tells us what kind.

Now that we have the fundamental idea of a Fruit class, add the peel/core idea as another field. If there are only these two choices maybe the field could be a boolean, "isPeelable". If there where, or could be in the future, other choices like "smash" or "pluck", now an enumeration might be a good idea, just like it is for the fruit type field. I suppose the class' instance field might be called "prepToEat"?

From here it gets interesting. Your design needs to be flexible to accommodate new fruit types. Also, it looks like you have a method for each "prepToEat" value. And we shall have none of that "exception coding" crap you describe in #1, #2 above.

Because each fruit has several dynamic parts we will create a factory class. This puts all the details for making all the different kinds - and more importantly, future code changes - into one class.

A KEY DESIGN ELEMENT is using a delegate for the "prepToEat" method. This again, prevents us from having to modify the Fruit class directly when we add fruits to our repitore.

  public class FruitEater
  {
     ArrayList<Fruit> myFruit;
     FruitFactory myFruitMaker;

     public FruitEater()
     {
        this.myFruit = new Arraylist();
        this.myFruitMaker = new FruitFactory();
     }

     public static void Main( args[] stuff )
     {
        myFruit.Add( myFruitMaker.Create( FruitType.Orange ));
        myFruit.Add( myFruitMaker.Create( FruitType.Apple ));

        foreach ( Fruit a in myFruit )
        {
           a.eat(); //FINALLY!!!!
        }
     }

  } //FruitEater class



  public class Fruit
  {
     public delegate void PrepareToEatDelegate();

     protected FruitType type;
     protected PrepType prepType;
     // pretend we have public properties to get each of these attributes


     // a field to hold what our delegate creates.
     private PrepareToEatDelegate prepMethod;

     // a method to set our delegate-created field
     public void PrepareToEatMethod( PrepareToEatDelegate clientMethod )
     {
        prepMethod = clientMethod;
     }

     public void Eat()
     {
        this.prepMethod();
        // do other fruit eating stuff
     }

      public Fruit(FruitType myType )
      {
        this.type = myType;
      }
  }

  public class FruitFactory
  {
     public FruitFactory() { }

     public Fruit Create( FruitType myType )
     {
        Fruit newFruit = new Fruit (myType);

        switch ( myType )
        {
           case FruitType.Orange :
              newFruit.prepType = PrepType.peel;
              newFruit.PrepareToEatMethod(new Fruit.PrepareToEatDelegate(FruitFactory.PrepareOrange));
              break;

           case FruitType.Apple :
              newFruit.prepType = PrepType.core;
              newFruit.PrepareToEatMethod( new Fruit.PrepareToEatDelegate( FruitFactory.PrepareApple ) );
              break;

           default :
              // throw an exception - we don't have that kind defined.
        }
        return newFruit;
     }// Create()

     // we need a prep method for each type of fruit this factory makes
     public static void PrepareOrange()
     {
        // whatever you do
     }

     public static void PrepareApple()
     {
        // apple stuff 
     }
  }// FruitFactory

  public enum FruitType
  {
     Orange
     ,Apple
     ,Grape
  }


  public enum PrepType
  {
     peel
     ,core
     ,pluck
     ,smash
  }

Answer 9

A lot of people have good suggestions, but let me add one other that doesn't require a complete redesign of code or the support of objects/classes in places (like sql) that obviously have no support for such things.

You stated:

Fruit fruit = acquireFruit();
if (fruit != Fruit.Orange && fruit != Fruit.Banana)
    coreFruit();
else
    pealFruit();
eatFruit();

Which will absolutely break in unexpected ways if Grapes are introduced. I think a better approach would be:

Fruit fruit = acquireFruit();
Boolean fruitPrepared = false;

if (fruit == Fruit.Orange || fruit == Fruit.Banana) {
    pealFruit();
    fruitPrepared = true;
}

if (fruit == Fruit.Apple) {
    coreFruit();
    fruitPrepared = true;
}

if (!fruitPrepared) 
  throw new exception();

eatFruit();

A third approach is very similar:

Fruit fruit = acquireFruit();

switch(fruit) {
  case Fruit.Orange:
  case Fruit.Banana:
    pealFruit();
    break;    
  case Fruit.Apple:
    coreFruit();
    break;
  default:
    throw new exception('unknown fruit detected');
    break;
}

Each of the above breaks in well defined ways when you've gone outside of what you've explicitly coded for. The main thing to take away is that you are explicitly doing something for a known condition as opposed to defaulting to something on an unknown condition. There's probably a better way of phrasing that.

Answer 10

Use the social factor!

enum Fruit
{ 
    Apple,
    Orange,
    Banana,
    // add Grape here, and I'll shoot you
    // not kidding.
}

With me it would work (i.e. make me study the application's inner design deep enough not to introduce changes based on "lightweight" assumptions only) :))