hackification.com

One of the “anti-patterns” I come across from time to time is over-use of enumerations for control flow. This article describes why I consider it an anti-pattern, and how to counteract it.

Let’s suppose we want to represent an arithmetic operation on two numbers – such as plus, minus, multiply, etc. We could represent that operation as an enum (although to be clear – I don’t think you should):

enum ArithmeticOperation
{
  Add,
  Subtract,
  Multiply,
  Divide
}

We could then define a function to apply this operation to two numbers (highlighting will be explained later):

int ApplyOperation(ArithmeticOperation op, int x, int y)
{
  switch(op)
  {
    case ArithmeticOperation.Add: return x + y;
    case ArithmeticOperation.Subtract: return x - y;
    case ArithmeticOperation.Multiply: return x * y;
    case ArithmeticOperation.Divide: return x / y;
    default: throw new InvalidOperationException();
  }
}

That’s… sort of fine – but what happens when we want to extend it?

Adding Operators: No Compile-Time Checking

Let’s consider what happens if we want to add another operator – say exponentiation. If we add the enum value but forget to implement the switch case value, everything compiles fine. It’s only once we come to run the code, and actually hit that code path, that we discover the mistake, giving a run-time exception.

Adding Methods: Bad Code Organisation

Next, imagine what happens if we need to add more methods that switch on this enum. We might need a method returning the operator symbol, and another returning the operator precedence.

string GetOperatorSymbol(ArithmeticOperation op)
{
  switch(op)
  {
    case ArithmeticOperation.Add: return "+";
    case ArithmeticOperation.Subtract: return "-";
    case ArithmeticOperation.Multiply: return "*";
    case ArithmeticOperation.Divide: return "/";
    default: throw new InvalidOperationException();
  }
}

int GetOperatorPrecedence(ArithmeticOperation op)
{
  switch(op)
  {
    case ArithmeticOperation.Add: return 10;
    case ArithmeticOperation.Subtract: return 10;
    case ArithmeticOperation.Multiply: return 20;
    case ArithmeticOperation.Divide: return 20;
    default: throw new InvalidOperationException();
  }
}

I’ve highighted the relevant code for the “add” operator. Notice how it’s spread all over the place. Switch statements might group by operation, but they split by concept – and that’s the exact opposite of what’s usually intended with OO coding.

A Better Way

I’d suggest a better way of approaching this problem is to use an abstract class.

abstract class ArithmeticOperation
{
  abstract int ApplyOperation(int x, int y);
  abstract string GetSymbol();
  abstract int GetPrecendence();
}

class AddArithmeticOperation : ArithmeticOperation
{
  override int ApplyOperation(int x, int y)
  {
    return x + y;
  }

  override string GetSymbol()
  {
    return "+";
  }

  override int GetPrecedence()
  {
    return 10;
  }
}

// ...and so forth for the other operators.

That way, all the code for each operator sits in the same place, and adding any new methods for an operator requires all operators to be implemented before the code will even compile.

When to Use Enums

So if enums should be avoided, why do most languages include them? Personally I’d be happy without them,  but I can think of one possible reason you might want to use an enum. If you’re defining a public interface designed for consumption by other coders, you could argue that simplifying the interface at the expense of the code inside is a valid trade-off.