Or you think that you do. Probably you're using it as a Factory or a Locator. Or you have a class with static methods that you use to load resources or something like that. Those are singletons right? No, but you thought they were. There are even some clever and occasionally useful Singletonish things implemented using ThreadLocals, but that's for later. The point is, you've used Singletons as an excuse for what amounts to a bunch of global variables and methods. Bad programmer! No copy and paste for you!
The real problem is in how you're using them. Singletons can't (easily or at all) be proxied, decorated, replaced or mocked. You're losing all kinds of flexibility, without gaining any sort of benefit. This is your code:
This is your code on inversion of control:
public void doSomething() {
MyConfig config = MyConfig.getConfiguration();
config.doSomething();
// etc....
}
private MyConfig config;Of course, somewhere else you need to
public void doSomething() {
config.doSomething();
// etc....
}
public void setConfig(MyConfig config) {
this.config = config;
}
myClassThatDoSomething.setConfig(configInstance);
, or have your container do it. This pattern is also called dependency injection, but unlike the Singleton, it's easy to get right. Don't call static lookup methods (or any lookup methods if you can avoid it) and make everything you need an object property. It's a simple change, but it gives you tons of flexibility. The only downside is that you have to learn how to use an IoC container. Fortunately, they're getting more idiot proof by the day, so with a little work, you can probably figure it out.Now, before you go and delete every last one of your singletons, there are a few situations where IoC wont work. For IoC to work, you have to have control of the creation of the object, or at least be able to get a reference to it before it's used. A good example is logging. Logging has to be available before the container even initalizes, so you can't dependency inject logging components (easily). And sometimes, the thing you need to inject can't be managed by the container. For example, suppose you need access to the current HTTP request object, or the session object? Ignore for a second that your container can usually mange to inject special objects like these and recognize that what makes these object special is that, in a particular scope, there is only one of them at a time. By their very definition, there can only be one HTTP Request and one Session at a time. To have two requests, or two session doesn't make any sense. That means you actually have a good candidate for a singleton. A filter like this can be very helpful (some types & casts omitted for brevity):
private static ThreadLocal request = new ThreadLocal();That lets anything that's executed inside the filter call MyFilter.getRequest() to get the current HTTP request object, regardless of whether the framework was kind enough to pass it in. Even this should only be a last resort. There may be a legitimate case where someone needs to wrap, replace or mock the request, but your singleton code gets in the way. How will you unit test code that accesses the request in this way? There is no real HTTP request when you're running unit tests. Dependency injection solves a lot of problems.
public static HttpRequest getRequest() {
return request.get();
}
public void doFilter(request,response,chain) {
request.set(request);
try {
chain.doFilter(request,response);
} finally {
request.clear();
}
}
Sometimes you can't avoid the Singleton, but considering the mess that you've made of things so far, maybe you should ask someone smarter than you next time you think you really need a Singleton. You probably don't. Now get back to work, and when you have some time, get the original (and best) design patterns book, read it, and stop abusing them. Design patterns are meant to describe good code, not magical pixie dust that you sprinkle on to make your crappy code suddenly good. Alas, that is another post.
No comments:
Post a Comment