Youssef13
Youssef13

Reputation: 4954

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

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?

Upvotes: 5

Views: 1028

Answers (2)

Sijmen
Sijmen

Reputation: 506

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.

Upvotes: 4

LeffeBrune
LeffeBrune

Reputation: 3477

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.

Upvotes: 3

Related Questions