developer.* Blogs - Open Comments Thread for "Factory Chain: A Design Pattern for Factories with Generics" by Hugo Troche - Comments

Ooops

Steve — Wed, 23 Mar 2005 06:01:02 -0800

Well, you probably wouldn't put the registration code in the constructor. And the classloader would still have to go to work before the object was needed. Not quite perfect then...

Registering with the factory

Steve — Wed, 23 Mar 2005 05:49:46 -0800

Registering with the factory seems like the way to go. It seems far more reasonable that a class would know the magic string to pull itself out of a hash, rather than know the next class in a sequence. And you could easily make the string a constant in the factory so it could be changed without too much work. Try this:

private static HashMap<String, Class<? extends Car>> map = new HashMap<String, Class<? extends Car>>();
public static Car getCar(String carType){
   try{
      Class<? extends Car> carClass = getHash().get(carType);
      return carClass.newInstance();      
   } 
   catch(Exception e) {e.printStackTrace();}
   return null;
}

public static HashMap<String, Class<? extends Car>> getHash(){
   return map;
}

public static void add(String type, Class<? extends Car> classObj){
   map.put(type, classObj);
}

private class Ford implements Car{
   public Ford(){
      CarFactory.add("Ford", getClass());
   }
}

private class Chevy implements Car{
   public Chevy(){
      CarFactory.add("Chevy", getClass());
   }
}

Trade off

htroche — Tue, 08 Mar 2005 10:07:30 -0800

Tim,
You are correct about the tradeoff. If you want compile-time safety, you need to load the classes before they are needed.

Even if we let the elements register themselves with the factory, you still have to hardcode the class object somewhere. There is no way around having somewhere in the code the class for the element hardcoded. We could make the factory more or less dynamic by allowing the elements to register themselves, and that is an option we should look into it.

Hugo

getHash() is a different problem

TimK — Tue, 08 Mar 2005 09:34:04 -0800

Hi, Hugo. Thanks for writing. There are several independent issues floating around. Your article presented a factory that could throw an exception, and you wanted to use generics in order to have the compiler generate code that would never throw an exception. I was trying to demonstrate a simpler way to achieve that. I specifically left getHash(), as in your original factory, even though I don't particularly like it, because I didn't need to touch it in order to keep the exception from being thrown.

The chain of factories is simply a hard-coded linked list. In this code, each class is responsible for identifying the class that comes after it. This is a breach of the single-responsibility principle: Each factory element is responsible for another class in the chain. In other words, while it's true that to add another class you only have to modify the last class in the chain, first you must determine which class that is! If you need a hard-coded list of hundreds of classes, it's still more maintainable to have this list in one place, rather than scattered among various unrelated classes.

Now, if you need to sift through hundreds of classes, it's better to use a hash (O(log N) time per invocation) than a linked list (O(N) time per invocation), because the hash is faster. I daresay, the hard-coded linked list, in the general case, is not as scalable as you seem to think it is.

Furthermore, whether it is a hash that references a bunch of Class objects or whether it's a linked-list that references a bunch of Class objects, the VM will still need to load these classes into memory, yes? As soon as we used the class static member, we told the VM to load in all these classes.

There seems to be a tradeoff: If you want compile-time safety, you need to load the classes before they're needed.

To decouple the factory from the factory elements we could use an observer pattern. Have each factory element register itself with the factory, which would then add that factory element to the hash. If your application allowed it, you could even load factory elements just before they're needed, reducing start-up and memory requirements.

Generic factory also comes into play here. I haven't worked out the code, but my guess is that you'd want to use generics in your factory-element superclass (as you did in your article) in order to decouple the factory code from the code of the created object.

-TimK

Just forgot to add to my last

htroche — Tue, 01 Mar 2005 15:07:41 -0800

Just forgot to add to my last comment. Using a factory chain is good scaling design decision, since you don't have big collections with all the elements floating around. Remember, you can't have all the elements of a generic factory defined in a file and then just lookup the file (or database table) when you need a particular element. So, if you are going to put all the element in a collection, you have to do so before the elements are needed.

Hugo

getHash() is the problem

htroche — Tue, 01 Mar 2005 14:52:14 -0800

The problem is how to implement getHash(). This example is simple enough so there is no problem adding more elements the hash manually. What if your factory could return a type from hundreds of elements? In that case the method getHash() will be hundreds of lines long. This will also mean having to modify the class CarFactory every time you have a new type of car. Which will also mean having to retest a class that will probably have plenty of dependencies.

By placing the reference to the next class in the factory into each element and creating a chain you only have to modify one small file: the last element in the factory. Also, the chain gives you the possiblity to reuse parts of the factory more easily.

Hugo

Not the simplest thing that could possibly work

TimK — Tue, 01 Mar 2005 11:51:55 -0800

The original factory class as implemented may generate a run-time exception. Consider this code from ClassicFactory.getCar(String):

// stuff elided ...
        try{
            String carClass = getHash().get(carType).toString();
            Class clazz = Class.forName(carClass);
            return (Car) clazz.newInstance();
        } catch(Exception e) {e.printStackTrace();}
        return null;

Actually, instead of printing a stack trace, getCar should probably throw an InvalidCarClassException or some such.

Doing the same cast in Java 5 loses you nothing. Moreover, casting the class returned by forName to a Class<? extends Car> also loses you nothing. It's just a different way to achieve the same result, only using generics. Big deal.

But generics can help us shore up our code, and without rewriting the entire factory, too. The key is in a subtle change reflected in the Factory-Chain code:
super.nextClass = Pan.class;
This code, taken from the Pot class, shows that in the Factory-Chain implementation, we are no longer using forName. Instead, we're referring directly to the class object. We can do this in the classic factory implementation, by using a HashMap containing Class objects, instead of containing String objects. Moreover, doing it this way means we would not have to maintain the Factory-Chain linked list manually, which is a maintainence nightmare waiting to happen.

But using generics, we should be able to go one better. (I haven't actually tested the following code. Please try it!)

public class CarFactory {
    public static Car getCar(String carType) {
        try{
            Class<Car> carClass = getHash().get(carType);
            return carClass.newInstance();
        } catch(Exception e) {e.printStackTrace();}
        return null;
    }

    public static HashMap<Class<Car>> getHash() {
        HashMap<Class<Car>> map = new HashMap<Class<Car>>();
        map.put("Ford", com.generic.factory.Ford.class);
        map.put("Chevy", com.generic.factory.Chevy.class);
        return map;
    }
}

The above code could of course be further streamlined.

In summary, the Factory-Chain pattern isn't really the best solution in this case. Using a chain of factories could be useful in other cases, for example, if multiple factories must be dynamically loaded and then behave as though they were components of the same factory.

-TimK

Open Comments Thread for "Factory Chain: A Design Pattern for Factories with Generics" by Hugo Troche

Daniel Read — Thu, 24 Feb 2005 19:31:03 -0800

This an open discussion thread for the developer.* Magazine article "Factory Chain: A Design Pattern for Factories with Generics" by Hugo Troche. If you haven't already, you can read it here, then add your comments below.