Extreme ASP.NET Makeover: Death of a Singleton-Testability
He’s Liked, But He’s Not Well Liked
If the current code works, why do we need to get rid of the singleton? What harm is it doing? After all, there’s a reason the pattern has a name. It must be useful.
We have a few issues with singletons. The easiest argument to make is that singletons make your code hard to test. If you make a call to a singleton in a class, then you must make sure the singleton is initialized whenever the test is run. If the singleton requires a connection to a database or the existence of a Web service, these must be configured prior to running the test. If you want reasons why this is a bad practice, look no further than our previous article on separation of concerns.
This lack of testability isn’t the only problem. After all, there are ways (usually neither easy nor pleasant) to make singletons testable. Indeed, ScrewTurn uses one of them by exposing a public constructor. Let’s look more closely at the AuthChecker class we’ve been working with. Why is it a singleton class? Perhaps so that its members can be accessed throughout the site without having to create a new instance of it? Perhaps to save on resources by reducing object allocation and garbage collection? (This would almost certainly be a premature optimization.)
Let’s look at the first public method on the class, CheckActionForGlobals. Where is this being used? If you trace its usages, you’ll find it is used in exactly three other classes: the AdminMaster master page and FileManager.ascx control (both in the WebApplication project) and the AuthorizationServices class in the Core project. (The latter is one of the refactorings we did in Part 6; originally, it was being called in Default.aspx in the WebApplication project.)
So this globally accessible method is really only being used in three places. Doesn’t sound all that global to us. One could make an argument that each of these classes could just create an instance of the class instead. That would make them easier to test and less fragile overall.
Of course, we’ve been cautiously selective in our example. CheckActionForGlobals is but one method in the AuthChecker singleton. If we look at all cases where we reference AuthChecker.Instance, we’ll find no less than 86 occurrences of it across 21 files.
“Aha!” you say. “Doesn’t that contradict your argument that it’s not really being used globally?” “Pshaw!” say we! What it does is highlight another issue with singletons. The Instance variable is used in 21 files, yet CheckActionForGlobals is used in 3. Another method, CheckActionForDirectory, is used in 5, not including calls made from within the AuthChecker class itself.
In short, our singleton has turned into a sort of catch-all bucket, a place to house disparate methods under the loosely-defined category of “authorization checking.” Granted, the AuthChecker class isn’t as messy as it could be, but the tendency with singleton classes is to make them more and more generic as you add “helper” methods to them. The ubiquitous Utils singleton often seen in many projects is a good example. Developers see a singleton and it’s just so darned tempting to drop a method in there rather than create a separate class with a more specific focus.
The result of those temptations is that you find code bases that are littered with unnecessary singletons. While they may seem innocuous at first, the problems we described earlier will start to haunt your project. To help you with those existing Singletons, let’s look at how we can move through our code and remove them.
Other videos from this article
- Demoing Different Singleton Implementations
- How Singletons Affect Testability
- Looking At AuthChecker
- Walkthrough Of The Entire Refactoring
Read the full article at http://msdn.microsoft.com/magazine/ee343987.aspx