Factory design pattern and violation of OCP (Open-Closed Principle)

Question

The factory in this tutorial obviously violates the OCP. Every time a shape is added to the system, we need to add it in the factory to support it. I'm thinking of another implementation and I'd like to know if there are any drawbacks.

public class ShapeFactory {

   //use getShape method to get object of type shape
   public Shape getShape(Class<? extends Shape> shapeType){
      return shapeType.newInstance();
   }
}

This implementation looks it doesn't violate OCP, and isn't complex. Is there any reason that I can't find any factory design pattern tutorial that mentions it?

Permeate

You might find this interesting softwareengineering.stackexchange.com/questions/302780/… – Agrostology

@Andreas, I can use .getConstructor method before .newInstance and pass the parameters to newInstance. – Permeate

@Agrostology it doesn't mention the implementation I'm asking about – Permeate

It doesn't mention it @Permeate because your implementation is not practical. As Andreas mentioned to be able to construct any non-trivial object you need to pass contructor parameters. If your factory is not aware of the types it creates, it by definition cannot know how to instantiate them. – Agrostology

Additionally, a factory typically hides concrete types from the client, but in your case your client has to call the factory with a concrete type. A bit of a contradiction. – Agrostology

"a factory typically hides concrete types from the client" That's a good point. Do you have a good implementation that doesn't violate OCP? The softwareengineering stackexchange question answers mention self-registering factories. But I don't know how to implement in code. – Permeate

@Permeate one thing to keep in mind is that refactoring is part of life. Before implementing feature, you want to be diligent but you don't want to spend too much time looking for every single edge case there will ever be. Most of the time, you will come up with a very SOLID design and yet, you might have to refactor something in the future. Often, people mistake this for violating Open/Close principle. My example, mentioned by LeffeBrune, makes this point. As was mentioned in that thread, a better solution could be Self-Registering Factories. BUT, it could argued that might be overkill. – Infringement

Sometimes "good enough" is good enough, if you catch my drift. – Infringement

Answer 1

I think the answer by @hfontanez to the question "Does the Factory Pattern violate the Open/Closed Principle?" covers. If you are adding new Shape subclasses you also somehow must add a way to create an instance of them. Let's assume you cannot modify the original ShapeFactory because it is a part of third party library, but you can subclass or decorate the original factory to add support for new shapes. Extending the example will look like:

public class AdvancedShapeFactory {
  private final ShapeFactory factory = new ShapeFactory();

  public Shape getShape(String shapeType) {
    if (shapeType.equalsIgnoreCase("PENTAGON")) {
      return new Pentagon();
    } else {
      return factory.getShape(shapeType);
    }
  }    
}

If the designer of the original fictional "Shapes" library wanted to make it easy to create new shapes by shape type they could implement a registry:

public class ShapeRegistry {
  private static final Map<String, Class<Shape>> shapeTypes = new HashMap<>();

  public void registerShape(String shapeType, Class<Shape> shapeClass) {
    shapeTypes.put(shapeType, shapeClass);
  }

  public Shape getShape(String shapeType) throws InstantiationException, IllegalAccessException {
    if (shapeTypes.containsKey(shapeType)) {
      return shapeTypes.get(shapeType).newInstance();
    }
    return null;
  }
}

It is worth to read about dependency injection and Guice as a good example.

Agrostology

Where the registerShape method should be called? – Permeate

I would expect that the library registers its own shapes when it is initialized. Your library that adds new shape types would also be responsible for registering them. – Agrostology

I tried to call it inside a static initializer, but the static initializer is never executed. Hence, nothing is registered. – Permeate

I've tried using reflections to do the registeration inside a static initializer in the factory itself. The client (caller) won't be depending on the subclasses or know about them, and it doesn't violate OCP. Are there any disadvantages to that? – Permeate

Nothing wrong with that, most dependency injection frameworks actually rely on reflection especially when combined with annotations. – Agrostology

@Agrostology I just saw this today. Thanks for the endorsement! – Infringement

Answer 2

There are a few drawbacks to this method.

Firstly, when the Class passed to getShape requires a constructor argument, the .newInstance will fail. For example:

public class Circle {
   public Circle(int diameter) {
      //something
   }
}

You could get into reflection by using getConstructor and figuring out what arguments to pass, but that is complex and error prone. And you lose type safety at compilation time. And how would the factory class know what values to pass to diameter?

One of the advantages of the factory design pattern is that the caller doesn't have to know what implementing class is used when calling. Take the following example:

public class ShapeFactory {
   public Shape getCircle(int diameter){
      return new Circle(int diameter);
   }
}

Whenever you call this method, the caller doesn't have need a dependency on the Circle class:

Shape s = ShapeFactory().getCircle(10);
s.draw();

In this way, only the ShapeFactory depends on the Circle. So when you change or replace the Circle class, only the ShapeFactory has to change.

To create make the shape program OCP compliant, we could replace the ShapeFactory with a dependency injection framework. The below code is pseudo-code that shows how this could work

// define the classes
class Circle {}
class Square {}

// for every class, provide a factory method. These do not have to exist inside a factory class.
public Circle createCircle() {
    return new Circle(10)
}

public Circle createSquare() {
    return new Square(42, 5)
}


public class Painter {
    //when a class requires an instance of the Circle class, the dependency injection framework will find `createCircle`, call it and add the result as an argument here.
    public Painter(Circle circle) {
       circle.draw();
    }
}


//when you need a painter object, we don't create it yourself but let the dependency framework do the heavy lifting
Painter p = dependencyframework.getInstanceOf(Painter.class)

There are many Java Dependency Injection frameworks but they all work something like this.

These frameworks do the exact same thing what you propose (things like newInstance and getConstructor, but a lot more of those), they just hide all the reflection complexity.

Brutify

This explains it very well. Can you add a good implementation that doesn't violate OCP to the answer? – Permeate

In this case implementing a perfect OCP-compliant factory yourself will hurt more then it gains. You would have to do a lot of reflection, lose type safety and introduce a lot complexity. That is probably not worth it. Take a look at dependency injection (softwareengineering.stackexchange.com/questions/220640/…), that might get you closer to an answer. – Brutify