Pregunta

abstract class Product
{
  public abstract Product createProduct();
  ...
}

class OneProduct extends Product
{
  ...
  static
  {
      ProductFactory.instance().registerProduct("ID1", new OneProduct());
  }
  public OneProduct createProduct()
  {
      return new OneProduct();
  }
  ...
}

class ProductFactory
{
  public void registerProduct(String productID, Product p)    {
      m_RegisteredProducts.put(productID, p);
  }

  public Product createProduct(String productID){
      ((Product)m_RegisteredProducts.get(productID)).createProduct();
  }
}

From : http://www.oodesign.com/factory-pattern.html The productID is used at two different places, inside the concrete Product and by the caller of the ProductFactory, How to keep the ProductID in sync at the two places?

Declaring all the productIDs in a separate class seems one option. What are other better ways to achieve this?

¿Fue útil?

Solución

The code you have shown and linked to is not a typical implementation of a factory. It seems the factory is here being used as a kind of dependency container, where classes can be registered to satisfy some dependency. The highly unusual parts are here that:

  • Each Product has a createProduct() factory method, implying that it's supposed to create itself. Usually, the creator and product are separate classes.

  • There can be only one singleton factory. This is entirely unnecessary, but was chosen so that…

  • The products register themselves at the singleton dependency container during class initialization. This was done to keep the factory “open” in the sense of the Open-Closed Principle, but misses the point. Because the Products are coupled to the singleton factory, the OCP is violated since we can't add another factory without changing all products.

    Far more importantly, this design only allows a single class to provide the product for some ID. If two products have the same ID, one would overwrite the other in the m_RegisteredProducts map. Since this is done during class initialization, the programmer cannot influence which class is chosen. If each ID is only provided by a single class, we actually have a 1:1 correspondence between IDs and classes.

User-provided IDs

For these reasons, I would expect the products to be registered at the factory during normal program execution. The programmer then has full control over the IDs used:

interface Product { Product createProduct(); }
class OneProduct extends Product {
  @Override
  public Product createProduct() { return new OneProduct(); }
}

class ProductFactory {
  private Map<String, Product> registry;

  public void register(String id, Product p) {
    registry.put(id, p);
  }

  public Product createProduct(String id) {
    Product creator = registry.get(id);
    if (id == null) throw ...;
    return creator.createProduct();
  }
}

public static void main(...) {
  ProductFactory factory = new ProductFactory();
  factory.register("one", new OneProduct());
  ...
  factory.createProduct("one");
}

This avoids any fragile reflection, and is simple and explicit. Since the programmer using the factory now has control over all IDs, they can use any desired scheme to enforce consistency, such as storing the used IDs as constants.

Class<T> as ID

In practice, these dependency containers are often queried to satisfy a dependency on any class that satisfies a given interface. For each managed interface, a concrete class implementing this interface can be registered. This allows us to use java.lang.Class objects as keys. The benefit is a lot more safety than using strings, since we can only reference classes that actually exist in the system (we can't statically force the class to be registered in the dependency container, though).

interface Creator<T> {
  T create();
}

interface Product { }
class OneProduct implements Product { }
class AnotherProduct implements Product { }

class Factory {
  Map<Class<?>, Creator<?>> registry = new HashMap<>();

  public <T> void register(Class<T> id, Creator<T> creator) {
    registry.put(id, creator);
  }

  public <T> T create(Class<T> id) {
    Creator<T> creator = (Creator<T>) registry.get(id);
    if (creator == null) throw ...;
    return creator.create();
  }
}

public static void main(...) {
  Factory factory = new Factory();
  // If I want to use AnotherProduct instead, I can just change it here
  factory.register(Product.class, new Creator<Product> {
    public Product create() { return new OneProduct(); }
  });

  ...

  Product p = factory.create(Product.class);
}

Note that a concrete type is registered under the interface Class<> object which it satisfies, rather than under its own ID. This allows a factory client to depend upon abstractions. If each object were registered under its actual type, we could just as well instantiate it directly with new.

Class<> objects are not the only objects we can use as ID aside from strings. We could also use enums, for example. However, and enum implies that we statically know all cases. It would then be better to list these cases separately instead of requiring some switch/case:

No IDs, just method names

If we will only ever instantiate a few known types through the factory, we don't have to use any IDs. Instead, we can use different method names in the factory. The benefit is that the compiler can now check that the factory provides the required type. The cost we pay for that safety is that we have to modify the factory for each new type we want to provide through it.

We can choose to implement this as an abstract factory:

interface ProductA {}
class OneProductA implements ProductA {}
class AnotherProductA implements ProductA {}

interface ProductB {}
class OneProductB implements ProductB {}
class AnotherProductB implements ProductB {}

interface AbstractFactory {
  ProductA createA();
  ProductB createB();
}

class OneFactory implements AbstractFactory {
  public ProductA createA() { return new OneProductA(); }
  public ProductB createB() { return new OneProductB(); }
}

public static void main(...) {
  AbstractFactory factory = new OneFactory();
  ProductA = factory.createA();
}

The biggest advantage of this approach is that the createX() methods can have different signatures in case the constructor needs to take arguments.

If we want to change which concrete class is provided for ProductA, we can subclass a concrete abstract factory:

class MyFactory extends OneFactory {
  public ProductA createA() { return AnotherProductA(); }
}

Statically checked dependency container

In my personal experience, it's often better to allow the create method to be swapped out individually, unless the abstract factory is needed to provide a consistent palette of objects. In most cases, I would hand-roll a dependency container like this:

interface ProductA {}
class OneProductA implements ProductA {}
class AnotherProductA implements ProductA {}

interface ProductB {}
class OneProductB implements ProductB {}
class AnotherProductB implements ProductB {}

@FunctionalInterface
interface Creator<T> {
  T create(DependencyContainer container);
}

class DependencyContainer {
  // using lambdas as more convenient notation
  public Creator<ProductA> createProductA = (container) -> new OneProductA();
  public Creator<ProductB> createProductB = (container) -> new OneProductB();

  public ProductA createProductA() { return createProductA.create(this); }
  public ProductB createProductB() { return createProductB.create(this); }
}

...
DependencyContainer container = new DependencyContainer();
// dynamically swap out any creator
container.createProductA = (container) -> new AnotherProductA();

ProductA product = container.createProductA();

Here, each creator object receives the dependency container as parameter. This allows dependencies between various products to be resolved internally. The shown solution is extremely flexible, while still allowing the existence of all required products to be checked statically. The container could in theory be subclassed to add new products, but existing products can be swapped out without having to subclass the whole dependency container, thus avoiding complex hierarchies (“prefer composition over inheritance”).

The only problem is that Java makes it difficult for the Creator<> interface to receive constructor parameters. Possible solutions include Object... varargs, an additional generic type parameter for an argument object (may be java.lang.Void), or returning a builder in place of the finished object. All of these are invisible to the outside since the createX() methods can wrap these details.

Counter-indications

These suggestions assume that you want as much static safety as possible, or that all IDs are at least only used internally. Some applications are different, and we have to map an ID originating outside our system to some class within our system. This might be the case if you want classes to be selected through configuration files. Using fully qualified class names (in analogy to Class<> objects as keys) would be possible, but can introduce security issues if malicious classes can be injected into the system. Also, renaming an otherwise internal class would break this interface.

It can be sensible to use conditionals or a switch/case to parse these IDs:

class ProductFactory {
  public Product create(String id) {
    if ("id1".equals(id)) return new OneProduct();
    throw ...;
  }
}

// a subclass extends the set of known IDs:
class ExtendedProductFactory extends ProductFactory {
  @Override
  public Product create(String id) {
    // override a base class choice
    if ("id1".equals(id)) return AnotherProduct();
    // add a new ID
    if ("id2".equals(id)) return SomeProduct();
    // let the base class handle the rest
    return super.create(id);
  }
}

We could also use a Map<String, Creator> as shown above, which would probably be easier to extend and is likely easier to debug.

Licenciado bajo: CC-BY-SA con atribución
scroll top