Is this Simple Factory violating the Open Closed Principle?

Question

Is this Simple Factory violating the Open Closed Principle?

The SimpleProductFactory needs to change every time a new concrete product needs to be created but it adheres to the single responsibility principle because that is the only reason it will ever change. Its sole purpose is so that the Client does not violate the open closed principle so I imagine it can't be a violation itself since obviously this code is needed somewhere.

I am not interested in changing the factory but whether this specific example is a violation or not.

Product

interface Product{
  public int getPrice();
}

Milk

class Milk implements Product{
  public int getPrice(){ return 5; }
}

Chips

class Chips implements Product{
  public int getPrice(){ return 3; }
}

SimpleProductFactory

class SimpleProductFactory{

  public Product createProduct(String productName){

    if(productName.equals("milk")){
      return new Milk();
    }
    else if(productName.equals("chips")){
      return new Chips();
    }
    return null;
  }
}

Client

class Client{
  public static void main(String[] args) {
    SimpleProductFactory productFactory = new SimpleProductFactory();
    Product prod = productFactory.createProduct("milk");
    System.out.println(prod.getPrice());

  }
}

Euphrates

Are you pointing to SimpleProductFactory class ? – Pullulate

@Pullulate I am not sure what you mean. If this was a real program I imagine I would have SimpleProductFactory used in multiple places but SimpleProductFactory would not have more methods added to it. – Euphrates

I think that you want to use an abstract factory for this. Edit: as Timothy Truckle mentions in his answer (1+) – Heavyhanded

I think what Ravi means is that if you inject a SimpleProductFactory into the Client rather than instantiate it directly, you can subclass the factory to change its behavior without violating OCP. – Unceremonious

Answer 1

Is this Simple Factory violating the Open Closed Principle?

To answer your questions. "Yes, Simple Factory violates the Open Closed Principle for a reason."

The Simple Factory pattern supposed to be modified in order to help us choosing specific class to the caller. If we make this class conforming to open closed principle then we have to shift burden to some other class and this class will not serve the purpose of factory anymore. Not all principles are absolute. We need to weigh the benefits when using or when not using.

Felishafelita

This is so precise! +1. Factory pattern is an exception to OCP on purpose. – Petrify

Answer 2

In addition to Timothy Truckle answer about the service locator...

In Java 8 you might want to use method references and the Supplier interface to implement a generic factory for such simple use cases like yours.

E.g.

class SimpleProductFactory {

    private Map<String, Supplier<? extends Product>> supplierRegistry = new HashMap<>();

    public void addProductSupplier(String productName, Supplier<? extends Product> productSupplier) {
        supplierRegistry.put(productName, productSupplier);
    }

    public Product createProduct(String productName) {
        Product product = null;

        Supplier<? extends Product> productSupplier = supplierRegistry.get(productName);

        if (productSupplier != null) {
            product = productSupplier.get();
        }

        return product;
    }
} 

And your client code will look like this

class Client{
  public static void main(String[] args) {
    SimpleProductFactory productFactory = new SimpleProductFactory();

    productFactory.addProductSupplier("milk", Milk::new); // Constructor reference
    productFactory.addProductSupplier("chips", Chips::new);

    Product prod = productFactory.createProduct("milk");
    System.out.println(prod.getPrice());

  }
}

As you can see the simple factory is

• open for extension, because you can simple add other product suppliers
• closed for modification, because you don't need to change it when another product is implemented you just add it.

PS: With a bit more refactoring you can simply turn it into a real generic factory for any type.

Mercator

Answer 3

The open/closed principle does not really apply to factories because after all they are the very source of the different typed objects...

On the other hand you could have an Abstract Factory looking up "real" factories using javas ServiceLoader. Then you could add more of that real factories even in their own jars without changing existing code...

Talcahuano

The OCP applies perfectly to both of the GoF factory patterns (Abstract Factory and Factory Method). It does not apply to the Simple Factory pattern because that was invented by Head First Design Patterns as a teaching aid. It is not intended for production code. – Unceremonious

@jaco0646, how does it apply to Factory Method? If you have new implementations they will have to be put somewhere, and this somewhere will thus have to be touched. – Sordid

@FelipeMartinsMelo The ServiceLoader looks up implementations of a given interface in the class path. The Code using a factory only needs to know the interface the factory implements (the abstact factory) and the interface the produced objects implement (both are the same for all concrete factories). You provide a new Factory by createing a file in the META-INF folder of the new jar having the FQN of the interface the factory implements (and the ServiceLoader looks for) as name containing the FQN of the new factory implementation as plain text. – Talcahuano

@jaco0646, yes, from this perspective I agree. – Sordid

Answer 4

The static factory example given above is still extendable for additional products like Icecream as follows. Even if we make the factory metod static still it can be extended (though not by conventionally overriding the method)

    class SimpleProductFactoryExt extends SimpleProductFactory {
        public Product createProduct(String productName) {
            if (productName.equals("Icecream")) {
                return new Milk();
            } else {
                return super.createProduct(productName);
            }
        }
    }

Coffle

Answer 5

Yes, it does, and it's appalling we see this design mistake over and over again, including several of the answers above.

The factory should externalise the place where it takes its list of products, eg, a config file that defines a product registry, a map of name => product class.

It should also have some register( Product ) method, so that extensions might tell the factory about new products (add Product.getName()).

A common way to deal with this is the SPI Interface, which allow extensions to list all the Product they add up, without having to register them (cause the SPI magic would take care of it).

Yet another variant is Spring, which allows for components of a given type to be auto-injected into a collection of that type.

The approach of Kedar of extending the initial factory using the chain of responsibility design pattern is also a decent design, which doesn't violate the open-close. However, in this case, you need something to tell your application which factory class it should use, eg, a Java property that can be set via -D when invoking your app, or, better, a .properties file in META-INF. I think SPI is similar (to the latter) and already there for you.

Whatever you choose, you should never ever write horrors like:

if ( key1 ) { code1 }
else if ( key2 ) { code2 }
... 

Especially where codei is always a variant of the same lines. This is verbose, poorly readable, error-prone, and yes, it forces you to touch the if/elseif chain to write extensions, so it definitely violates the open-close. Indeed, it's the typical example that textbooks take to explain the benefits of such principle.

In general, the decent way to write the same is something like:

Map<key, Case> cases;

// Populate cases, possibly from the outside and in extensible way

void init ( key ) {
  Case case = cases.get ( key )
  // Use elements in 'case' (getters, methods, lambdas) to do what's needed
  // that is, write code-i as parameterised over case
}

In general, Case is a proxy for the class you need to build and/or config based on key (eg, ProductBuilder, ProductConfig). In a simpler situation, you can use the class directly (ie, Product).

A variant of the latter might simply be: Product product and then product is instantiated dynamically, taking the class' (FQN) name to instantiate from some external source (eg, -D property or .properties). For instance, this is the typical way to obtain a JDBC driver.

Gustatory