Code needs access to an object but doesn't need a global point of access to it.
Move the Singleton's features to a class that stores and provides access to the object. Delete the Singleton.
Motivation Singletonitis, a term I coined, means "addiction to the Singleton pattern." The intent of a Singleton is to "ensure a class only has one instance, and provide a global point of access to it" [DP, 127]. You're infected with Singletonitis when the Singleton pattern fixes itself so deeply into your skull that it begins lording it over other patterns and simpler design ideas, causing you to produce way too many Singletons. I've rid myself of Singletonitis and am now considering starting "Singletons Anonymous," a place where recovering Singleton abusers can support each other on the slow journey back to using simple, nonglobal objects. The Inline Singleton refactoring is a useful step on that journey. It helps rid your systems of unnecessary Singletons. This leads to the obvious question: When is a Singleton unnecessary? Short answer: Most of the time. Long answer: A Singleton is unnecessary when it's simpler to pass an object resource as a reference to the objects that need it, rather than letting objects access the resource globally. A Singleton is unnecessary when it's used to obtain insignificant memory or performance improvements. A Singleton isn't necessary when code deep down in a layer of a system needs to access a resource but the code doesn't belong in that layer to begin with. I could go on. The point is that Singletons aren't necessary when you can design or redesign to avoid using them. When writing this refactoring, I decided to ask Ward Cunningham and Kent Beck to comment on the Singleton pattern. Ward Cunningham on Singletons So much of the Singleton pattern is about coaxing language protection mechanisms into protecting this one aspect: singularity. I guess it is important, but it seems to have grown out of proportion. There is a proper context for every computation. So much of object-oriented programming is about establishing context, about balancing the lifetimes of variables, causing them to live the right length of time and then die gracefully. I'm not afraid of a few globals. They provide the global context that everyone must understand. There shouldn't be many though. Too many would scare me. |
Kent Beck on Singletons The real problem with Singletons is that they give you such a good excuse not to think carefully about the appropriate visibility of an object. Finding the right balance of exposure and protection for an object is critical for maintaining flexibility. Massimo Arnoldi and I once worked on a system that had a Singleton for storing exchange rates. Every time we wrote a test that handled multiple currencies, we had to make sure to save the old exchange rates, store some new ones, run the test, then restore the old exchange rates. One day we got fed up with test mistakes that were caused by using the wrong exchange rates. "But the exchange rates are used all over the system!" we whined. A good idea is a good idea, though, so we looked at all the places where the exchange rates were used. We added parameters as necessary to pass the exchange rates explicitly. We thought it would be a huge amount of work, but it only took us half an hour. Sometimes it was a little difficult to get the exchange rates to where they were needed, but it was obvious how to refactor to make it easy. These refactorings also resolved several chronic design problems that had bothered us but we hadn't known how to tackle. The result of the half hour: |
Martin Fowler also acknowledges the need for a few globals, though he uses them only as a last resort. His Registry pattern, from Patterns of Enterprise Application Architecture, is a slight variation on Singleton. Martin describes a Registry as "a well-known object that other objects can use to find common objects and services" [Fowler, PEAA, 480]. Regarding when to use this pattern, he writes: There are alternatives to using a Registry. One is to pass around any widely needed data in parameters. The problem with this is that parameters are added to method calls when they aren't needed by the called method but only by some other method that's called several layers deep in the call tree. Passing a parameter around when it's not needed 90 percent of the time is what leads me to use a Registry instead. . . . So there are times when it is right to use a Registry, but remember that any global data is always guilty until proven innocent. [Fowler, PEAA, 482–483] My rather close reading of Design Patterns [DP] infected me with Singletonitis. Every pattern in that book contains a Related Patterns section, and in many of those sections you'll find sentences that mention Singleton. For example, in the section on the State pattern, the authors write, "State objects are often Singletons" [DP, 313], and in the section on the Abstract Factory pattern, they write, "A concrete factory is often a Singleton" [DP, 95]. In defense of the authors, these sentences simply observe that State and Abstract Factory classes often are Singletons. The book doesn't say they have to be. If there is a good reason to make a class a Singleton or Registry, do so. The refactoring Limit Instantiation with Singleton (296) describes a good reason to refactor to a Singleton: real performance improvement. It also cautions against premature optimization. One thing is certain: you need to think and explore really hard before you implement a Singleton. And if you encounter a Singleton that shouldn't be a Singleton, by all means inline it! Benefits and Liabilities | + | Makes object collaborations more visible and explicit. | | + | Requires no special code to protect a single instance. | | – | Complicates a design when passing an object instance through many layers is awkward or difficult. |
|
Mechanics This refactoring's mechanics are identical to those of Inline Class [F]. In the following steps, an absorbing class is one that will take on the responsibilities of the inlined Singleton. - 1. Declare the Singleton's public methods on your absorbing class. Make the new methods delegate back to the Singleton, and remove any "static" designations they may have (in the absorbing class).
If your absorbing class is itself a Singleton, you'll want to keep the "static" designations for the methods.
- 2. Change all client code references to the Singleton to references to the absorbing class.
- 3. Use Move Method [F] and Move Field [F] to move features from the Singleton to the absorbing class until there is nothing left.
As in step 1, if your absorbing class is not a Singleton itself, remove any "static" designations from the methods and fields you move.
- 4. Delete the Singleton.
Example The code for this example comes from an early evolution of a simple, console-based blackjack game. The game is played on a console by displaying a player's cards, repeatedly prompting a player to hit or stay, and then displaying the player's hand and the dealer's hand to reveal who won. Test code can run this game and simulate user input, such as hitting and staying during game play. A player's simulated input is specified and obtained at runtime from a Singleton called Console, which holds on to one instance of HitStayResponse or one of its subclasses: public class Console { static private HitStayResponse hitStayResponse = new HitStayResponse(); private Console() { super(); } public static HitStayResponse obtainHitStayResponse(BufferedReader input) { hitStayResponse.readFrom(input); return hitStayResponse; } public static void setPlayerResponse(HitStayResponse newHitStayResponse) { hitStayResponse = newHitStayResponse; } }
Prior to starting game play, a particular HitStayResponse is registered with the Console. For example, here's some test code that registers a TestAlwaysHitResponse instance with the Console: public class ScenarioTest extends TestCase... public void testDealerStandsWhenPlayerBusts() { Console.setPlayerResponse(new TestAlwaysHitResponse()); int[] deck = { 10, 9, 7, 2, 6 }; Blackjack blackjack = new Blackjack(deck); blackjack.play(); assertTrue("dealer wins", blackjack.didDealerWin()); assertTrue("player loses", !blackjack.didPlayerWin()); assertEquals("dealer total", 11, blackjack.getDealerTotal()); assertEquals("player total", 23, blackjack.getPlayerTotal()); }
The Blackjack code that calls the Console to obtain the registered HitStayResponse instance isn't complicated code. Here's how it looks: public class Blackjack... public void play() { deal(); writeln(player.getHandAsString()); writeln(dealer.getHandAsStringWithFirstCardDown()); HitStayResponse hitStayResponse; do { write("H)it or S)tay: "); hitStayResponse = Console.obtainHitStayResponse(input); write(hitStayResponse.toString()); if (hitStayResponse.shouldHit()) { dealCardTo(player); writeln(player.getHandAsString()); } } while (canPlayerHit(hitStayResponse)); // ... }
The above code doesn't live in an application layer that's surrounded by other application layers, thereby making it hard to pass one HitStayResponse instance all the way to a layer that needs it. All of the code to access a HitStayResponse instance lives within Blackjack itself. So why should Blackjack have to go through a Console just to get to a HitStayResponse? It shouldn't! It's yet another Singleton that doesn't need to be a Singleton. Time to refactor. - 1. The first step is to declare the public methods of Console, the Singleton, on Blackjack, the absorbing class. When I do this, I make each method delegate back to the Console, and I remove the "static" designation for each method:
public class Blackjack... public static HitStayResponse obtainHitStayResponse(BufferedReader input) { return Console.obtainHitStayResponse(input); } public static void setPlayerResponse(HitStayResponse newHitStayResponse) { Console.setPlayerResponse(newHitStayResponse); }
- 2. Now I change all callers to Console methods to be callers to Blackjack's versions of the methods. Here are a few such changes:
public class ScenarioTest extends TestCase... public void testDealerStandsWhenPlayerBusts() { Console.setPlayerResponse(new TestAlwaysHitResponse()); int[] deck = { 10, 9, 7, 2, 6 }; Blackjack blackjack = new Blackjack(deck); blackjack.setPlayerResponse(new TestAlwaysHitResponse()); blackjack.play(); assertTrue("dealer wins", blackjack.didDealerWin()); assertTrue("player loses", !blackjack.didPlayerWin()); assertEquals("dealer total", 11, blackjack.getDealerTotal()); assertEquals("player total", 23, blackjack.getPlayerTotal()); }
and
public class Blackjack... public void play() { deal(); writeln(player.getHandAsString()); writeln(dealer.getHandAsStringWithFirstCardDown()); HitStayResponse hitStayResponse; do { write("H)it or S)tay: "); hitStayResponse = Console.obtainHitStayResponse(input); write(hitStayResponse.toString()); if (hitStayResponse.shouldHit()) { dealCardTo(player); writeln(player.getHandAsString()); } } while (canPlayerHit(hitStayResponse)); // ... }
At this point I compile, run my automated tests, and play the game on the console to confirm that everything is working correctly.
- 3. I can now apply Move Method [F] and Move Field [F] to get all of Console's features onto Blackjack. When I'm done, I compile and test to make sure that Blackjack still works.
- 4. Now I delete Console and, as Martin recommends in his Inline Class [F] refactoring, I hold a very short but moving memorial service for another ill-fated Singleton.
|
