One of the attendees of the recent design improvement workshop that I ran sent me a question today. I've taken the liberty of changing the code a bit to demonstrate the example, but the intent is basically the same.
When we did the design improvement workshop we discussed how static methods can be a code smell. I'm wondering is you think this holds true for static factories since I don't feel right about creating a new instance of a factory just to ask it for an object ?
For example, I would normally have a factory like this:
public class ThingFactory {
public static Thing create() {
// ...
}
}
So the calling code would look like this:
public class ThingProcessor {
public void process() {
Thing myThing = ThingFactory.create();
// ...
}
}
I'm a little anxious about code that looks like this:
public class ThingProcessor {
public void process() {
Thing myThing = new ThingFactory().create();
// ...
}
}
Since we create two object's and only one is kept.
The problem with static methods is that they make unit testing harder. You can't override them in a subclass, and you can't mock them out. So the implementation you've got is the only one that can be called. Here's how I would implement the factory from the example above:
public interface ThingFactory {
public Thing create();
}
public class DefaultThingFactory implements ThingFactory {
public Thing create() {
// ...
}
}
Now that I have an interface, I've got a mechanism that allows me to mock a method call. A useful trick to use in conjunction with that is to allow the actual implementation to be used by default in production code. You can do that with a combination of constructors like this:
public class ThingProcessor {
private final ThingFactory _factory;
// This constructor lets you inject a mock for testing.
public ThingProcessor(ThingFactory factory) {
_factory = factory;
}
// This constructor can be used by production code that just wants the default factory.
public ThingProcessor() {
this(new DefaultThingFactory());
}
public void process() {
Thing myThing = _factory.create();
// ...
}
}
The second design is much easier to test, yet the API for users of the ThingProcessor is exactly the same.
Read: Changing static factories to a testable design.