Creating instance depending on enum parameter

Question

For example I have classes Apple, Orange , Banana , etc.. inherited from Fruit . Also I have enum Fruits

enum Fruits
{
    Apple,
    Orange,
    Banana
}

I want to create correct instance depending on enum parameter.

I know about simple factory.

public Fruit CreateFruit(enum Fruits fruits)
{
    switch (fruits)
    {
        case Apple:
            return new Apple();
        case Orange:
            return new Orange();
        case Banana:
            return new Banana();
    }
}

But this method violates the Open-Closes Principle (OCP), so are there better solutions?

Answer 1

There is no real way to avoid such a factory while using an enum. But you could replace the enum with an factory object, something like:

public interface IFruit { }
public class Apple: IFruit { }
public class Orange : IFruit { }

public interface IFruitFactory
{
    IFruit Create();
}

public class AppleFactory : IFruitFactory
{
    public static readonly AppleFactory Instance = new();
    public IFruit Create() => new Apple();
}
public class OrangeFactory : IFruitFactory
{
    public static readonly OrangeFactory Instance = new();
    public IFruit Create() => new Orange();
}

With this pattern you would pass around a IFruitFactory instead of a fruit enum, allowing anyone to easily constrct a fruit from it. If you want to add a Banana you just create a new BananaFactory.

But I would recommend being pragmatic. The problem with switch-methods as in your example is when someone forgets to add a case when adding a new value, putting the factory close to the enum declaration, i.e. in the same file, help reduce this risk.

As is often the case, rules like 'open-close principle' are general recommendations, but you need to know why they exist and when they are applicable. Trying to dogmatically follow all design principles is just not a good idea.

Answer 2

in order to make instance of each class based on enum value, you should use attribute. for example you can add this attribute :

  public class FruitAttribute : Attribute
  {
      public Type Type { get; }

      public FruitAttribute(Type type)
      {
         Type = type;
      }
  }

then you should define value of this attribute for your enum:

  enum Fruits
  {
      [FruitAttribute(typeof(Apple))]
      Apple,

      [FruitAttribute(typeof(Orange))]
      Orange,

      [FruitAttribute(typeof(Banana))]
      Banana
  }

now you can rewrite your CreateFruit method like this:

  Fruit CreateFruit(Fruits fruits)
  {
      var type = fruits.GetType();
      var memberInfo = type.GetMember(fruits.ToString())[0];
      var fruitAttribute = memberInfo.GetCustomAttributes<FruitAttribute().First();
      return (Fruit)Activator.CreateInstance(fruitAttribute.Type);
  }

Answer 3

Disclaimer: this is not an actual answer, just a very long comment about a question which should have been closed because of being to opinion-based in the first place.

As with many principles, the Open-Closed-Principle is rather a guideline than a fix law. Sometimes you need to bend the rules a bit, sometimes you need to break them. The practical question rather is: where do you do that. You cannot extend a system without any kinds of modification to it. Even a config-file - e.g. for a DI-container - is some sort of code which you need to change in order to modify your system.

Having said this you usually need to apply the changes to your system somewhere. In your case you can of course introduce further abstractions - e.g. a factoy-pattern or even an abstract factory. You can rely on a DI-container, or some kind of a naming-convention and some reflection to instantiate your types. However that just introduces a further level of abstraction without solving the actual question: how to define which fruit to create?

Depending on how often and extensivly you extend your system, your naive switch may be pretty fine. If you never add further types, why bother about the factory-method? But even if you add some types, you need to change only the factory, not the rest of your system, so the rest is still closed about modifications. One may read the principle also as "there's exactly one single place for createing types in my code, so the rest of my system is completely independend on them". Of course that place violates some principles, but that may be okay - in particular when this gluecode is auto-generated as in a DI-container for instance.

So long story short: you have allways a trade-off here between practical use and abstract design. There's no general way how to balance between these.

To give this comment a bit more of an answer - even an opinion-based one, I add my favourit idea here. Instead of hard-coding the types that relate to your enum, you can introduce some convention, e.g. forcing enum-value and type-name to be equivalent:

enum Fruits
{
    Apple,
    Banana,
    Orange
}

Fruit CreateFruit(Fruits fruit)
{
    var typeName = fruit.ToString();
    ...
    // apply assembly-qualified typeName
    ...
    return (Fruit) Activator.CreateInstance(typeName); 
}

Answer 4

As a general observation, avoid enum types in C#. From an object-oriented perspective, there's no reason to have them, as explained by Martin Fowler in Refactoring. This book suggests that 'type codes' (as it's called in the book, which has Java examples) are a code smell, and suggests various refactorings/design changes that are more object-oriented - essentially making good use of polymorphism instead.

In addition to that, C# enum types aren't type-safe. At run time they're just glorified integers, and it's entirely possible that a type like Fruits may actually have the value 42.

All that said, however, at the boundaries, applications aren't object-oriented anyway. You often run into situations where you receive data from the outside, and you'd like to translate it into a polymorphic type like Fruit. Even if an enum may not be the best way to model that, you may receive external input that suggests which type of data is in effect. You may receive a CSV file or JSON object where the type is indicated as a string; e.g. "apple", "orange", or "banana". Alternatively, you may receive integer codes such as 1, 2, and 3.

What remains now is a parsing problem. You are, however, running into a variation of the expression problem, because, regardless of how you slice or dice the problem, a primitive input type like a string or an integer implies a set partition. For example, if your input identifies the type as a string, you have an implicit partition of all strings into "apple", "orange", "banana", and their complement: all other strings.

Practically, it means that a parser may look like this:

public Fruit? TryParse(string candidate)
{
    switch (candidate)
    {
        case "apple":
            return new Apple();
        case "orange":
            return new Orange();
        case "banana":
            return new Banana();
        default:
            return null;
    }
}

Notice that you have four cases, not three.

If you add a new subtype, you'd have to change this implementation.

Now, there are ways to move this kind of problem around. For example, Code Complete suggests a technique called table-driven methods. You could, for example, implement your parser based on a dictionary:

public Fruit? TryParse(string candidate)
{
    if (dict.TryGetValue(candidate, out var fruit))
    {
        return fruit;
    }
    return null;
}

While this seems to solve the original problem with the Open-Closed Principle (OCP), you still need to populate that dictionary somewhere.

You may decide to populate such a dictionary by reading configuration data from somewhere (e.g. a configuration file), but beware of the The Configuration Complexity Clock or the inner-platform effect. Is it actually worth it to go to such lengths?

The five SOLID principles are all guidelines that help you address certain problems related to maintenance of code bases (see e.g. APPP). If you don't have those problems, there's no reason to apply a concept like the OCP.

You'd typically like to avoid an anti-pattern like Shotgun Surgery, so you don't want to have switch/case expressions all over your code base. On the other hand, refactoring to a single parser at least follows the Single Responsibility Principle in that the only reason you'd have to change the parser is if the enumeration of subtypes changes. Furthermore, there's only one place you'd need to implement such a change.

Finally, the conclusions posited by the expression problem has been challenged. For a C# perspective on that, see my reading of the paper Extensibility for the Masses.