Factory which breaks Open-closed principle

Question

Classical implementation of Abstract Factory/Factory method is based on polymorphism. But on the lowest level we have to have some kind of factory which is not based on polymorphism and breaks open-closed principle:

public Device Create(string deviceName)
{
  switch (deviceName)
  {
    case "Device1": return new Device1();
    case "Device2": return new Device2()'
    default: throw new NotSupportedDeviceException(deviceName);
  }
}

Is there any special name for this kind of factories?

score 8 · Accepted Answer · answered Jun 22 '11 at 23:03

From what I can see, you've posted a perfectly valid example of the Factory Method pattern:

Both Device1 and Device2 inherit/implement the Device class/interface - polymorphism is still a key aspect of this example.
You're using the Factory Method to encapsulate the logic to determine which concrete class to instantiate and return.
The caller of the factory method remains nicely ignorant of the concrete classes involved - he just knows he gets back a Device. Which is exactly what you want a Factory Method to do for you.

It's true, the internal implementation is a little heavy-handed (switch-case). However, that doesn't make it any less of a true Factory Method pattern.

I don't really see where your example "isn't based on polymorphism" nor where it "breaks the open/closed principle". If I've missed the point entirely, feel free to update your post, to help us zero in on your question.

Now if the Factory method was taking the passed deviceName, and using it to find an exact match of the concrete class to instantiate (using reflection), that would absolutely break the Factory Method pattern, because the caller would have to have intimate knowledge of the different concrete classes. But as currently written, the passed deviceName is just a piece of data used in the decision-making process - it doesn't break the encapsulation of the factory pattern.

It breaks the open/closed principle couldn't be as clear as that, any time you need to add a new Device you have to edit Create and this breaks the Open/Closed — Rosmarine Popcorn, Mar 12 '15 at 14:49
I have the same opinion, it breaks the OCP. A good implementation is from the @brahmagupta example. — ovonel, Jun 18 '15 at 12:22

score 7 · Answer 2 · answered Jun 23 '11 at 12:13

public class Factory
{
   private Map<String, Device> devices;

    public Factory(Map<String, Device> devices)
    {
        this.devices = devices;
    }

    public Device Create(String deviceName)
    {
         return devices.get(deviceName);
    }
}

Configure the map using dependency injection.

score 3 · Answer 3 · answered Jun 18 '11 at 18:15

You are talking about the AbstractFactory Factory, right? One way to avoid what you're talking about is through configuration. If the correct factory is chosen based on some parameters or setup, then you don't necessarily have this issue. For example, each Device your software is deployed on would specify a different concrete instance to use. This could be done manually or through dependency injection. If it really needs to be a runtime decision, then I am not sure what else you could do.

score 2 · Answer 4 · edited May 23 '17 at 11:52

If you want to respect the open-closed principle in your function by allowing the factory to be extensible, then you can use the dependency injection pattern to configure the factory as shown in the answer by brahmagupta.

To answer the specific question as to whether there is a special name for the pattern you showed, the answer is no. It is still considered an example of the factory method pattern.

score 0 · Answer 5 · answered Jun 25 '11 at 13:30

This is indeed a Factory Method and there's nothing wrong with using concrete classes as their construction logic may vary and that's where Factory can be useful.

But in this particular case you can use reflection or IoC containers to achieve low coupling.

For example, if you're using C# and Common Service Locator abstraction, you can then register your Device implementations with names and then you wouldn't even need a factory at all, because it'd look like:

public Device Create(string deviceName)
{
    return ServiceLocator.Current.GetInstance<Device>(deviceName);
}

Factory which breaks Open-closed principle

5 Answers5

Linked