Singleton Smells
By: Jan-Kees van Andel, 19 April 2009
I really hate the Singleton pattern. It makes code hard to test, it has nothing to do with general OO principles and the Singletons I see daily (written by others of course 
) usually look very procedural.
People who use Singletons in Java all the time have more fundamental problems (I’m not talking about singleton scope in Spring btw), so I’m not gonna rant about that problem here, but I do like to show you an alternative which makes it easier to unit test Singletons.
Neal Ford published a talk on InfoQ called: “10 Ways to Improve Your Code”. Item 4: “Good citizenship” is about static methods and especially the Singleton design pattern. Static methods have their usage, for example simple stateless utilities like: int Math.sqrt(double) or Apache Commons. But when you’re going to add state into the mix, things get messy.
Singleton usage
Let’s step back. Why are we using Singletons in the first place?
- First, as the name says, because you want to guarantee that there is only one instance. Typical examples in the Java libraries are java.awt.Toolkit and java.io.Console.
For those of you interested, Console doesn’t have a static getConsole method, but can be accessed through System.console() and – in the Sun implementation – SharedSecrets.getJavaIOAccess().console(). In these cases, a Singleton is needed because there may NEVER be two instances of these classes. Otherwise strange things will happen, because you never know which instance contains the state you need. - Second, and depending on the circumstances this is a good or a bad thing, Singletons are the Object Oriented equivalent of globals. Almost everybody agrees that (non-immutable) globals are very bad, but for some reason, many developers still use Singletons. But, for certain objects, it’s useful to be able to access them from everywhere, without needing parameters all the time.
And of course, there are plenty of myths regarding Singletons, for example because singletons can be lazily instantiated on first use. Well, guess what? Classes are lazily initialized by the ClassLoader, so unless you’re messing with cyclic initialization dependencies you don’t need any custom lazy loading mechanism.
By the way, lazy initialization is a bad thing that should be avoided if possible. See Joshua Bloch’s Effective Java for more details.
An example Singleton
Below is a snippet with an example Singleton. You probably see Singletons like this all day.
public final class BadSingleton { private final String prop1; private final String prop2; private BadSingleton() { this.prop1 = System.getProperty("prop1"); this.prop2 = System.getProperty("prop2"); } private static final BadSingleton INSTANCE = new BadSingleton(); public static BadSingleton getInstance() { return INSTANCE; } // Other methods }
It’s just a simple Singleton. It uses field initialization for thread safety and simplicity, but is still lazily initialized (by the ClassLoader). Further, initialization is done in the constructor where the environment is queried for some settings.
As you might know, this code is not testable, not even with reflection/setAccessible(true) hacking, since the field is declared static + final. See the following snippet for a misguided attempt to create a unit test:
public class BadSingletonTest { @Test public void testGetInstance_Fail() throws Exception { Constructor<BadSingleton> constructor = BadSingleton.class.getDeclaredConstructor(); constructor.setAccessible(true); BadSingleton obj1 = constructor.newInstance(); Field field = BadSingleton.class.getDeclaredField("INSTANCE"); field.setAccessible(true); field.set(null, obj1); // Exception because INSTANCE is static + final BadSingleton obj1 = BadSingleton.getInstance(); BadSingleton obj2 = BadSingleton.getInstance(); Assert.assertSame(obj1, obj2); } }
An alternative for this issue would be lazy initialization, but this has the disadvantages of not being thread safe when not synchronized correctly. Java Concurrency in Practice by Brian Goetz is a must-read when you are interested in thread safety issues.
Alternative using a factory
This is the alternative Singleton class. It’s a modified version of Neal’s example.
public final class GoodSingleton { private final String prop1; private final String prop2; private GoodSingleton(String prop1, String prop2) { this.prop1 = prop1; this.prop2 = prop2; } // Other methods }
The responsibility of instantiating the Singleton is programmed into the Factory, which is shown below:
public class GoodSingletonFactory { private static final GoodSingleton INSTANCE; static { try { String prop1 = System.getProperty("prop1"); String prop2 = System.getProperty("prop2"); Constructor<GoodSingleton> constructor = GoodSingleton.class.getDeclaredConstructor(String.class, String.class); constructor.setAccessible(true); INSTANCE = constructor.newInstance(prop1, prop2); } catch (Exception e) { throw new RuntimeException("error initializing " + GoodSingletonFactory.class.getName(), e); } } public static final GoodSingleton getInstance() { return INSTANCE; } }
As shown in the snippet, the factory uses reflection to access the private constructor. Note that this trick only works if there is no SecurityManager or if the active SecurityManager approves this action.
It’s also a little bit less “Singleton” since it is possible to have more than one instance. The same reflection trick we used, now works a bit against us. If this is not an issue for you, it doesn’t matter. After all, developers who want to abuse your API always succeed if they really want to.
But we do get the advantage of better testability, as shown in the following two snippets:
public class GoodSingletonTest { @Test public void testConstructor_Ok() throws Exception { Constructor<GoodSingleton> constructor = GoodSingleton.class.getDeclaredConstructor(String.class, String.class); constructor.setAccessible(true); GoodSingleton obj1 = constructor.newInstance("1", "2"); // Assert stuff } }
public class GoodSingletonFactoryTest { @Test public void testGetInstance_Only_one_instance() throws Exception { System.setProperty("prop1", "test value"); System.setProperty("prop2", "test value"); GoodSingleton instance1 = GoodSingletonFactory.getInstance(); Assert.assertNotNull(instance1); GoodSingleton instance2 = GoodSingletonFactory.getInstance(); Assert.assertNotNull(instance2); Assert.assertSame(instance1, instance2); } }
As you can see, testing now becomes trivial.
Notes
Choosing the factory approach has some effects that may be important to you. Here’s a list of consequences and random thoughts.
- When using the reflection approach, it is possible for developers to mess up the Singleton behavior using reflection. When they do this, you may end up with multiple instances, which may be undesirable. When using a Singleton and the instance field is declared as static + final, this is not possible. The importance of this point differs per situation.
- You can choose to make the constructor package private (default) accessible. This way, you don’t need reflection to access it from the same package. If you put the Factory and the unit tests in the same package, this will make the code much more readable, especially for less experienced developers. It’s also less error prone, because the compiler isn’t capable of doing thorough checks on reflective code. On the other hand, if a developer put’s a custom class in the same package, it’s possible to invoke the constructor and thus create multiple instances.
- There is a myth that reflection is slow. It’s true that reflection is not as fast as JIT optimized (plain Java) bytecode. But since about 2000, there have been massive improvements in reflective invocation performance. And of course, if you look at the snippets, reflection is only used the first time.
- Using a static initializer block in the Factory and an immutable (all fields final, no lazy initialization) Singleton class makes the entire structure less sensitive to Concurrency hazards. Again, see Brian Goetz’ book for details.
If you ask me, the advantages outweigh the disadvantages by miles, especially when you consider that when developers misbehave (like in the first two bullets) in such a way that they break the Singleton principle, they should (and on my projects will) be ass whooped.
19 April 2009 om 7:34 pm
Ah, ik begrijp nu waarom je bij het stuk van Frank over cyclische afhankelijkheden met dat ranzige reflectie-voorbeeld kwam!
Ook al is reflectie tegenwoordig snel, dan is dat nog geen excuus om het te gebruiken.
Als je per se reflectie wil gebruiken, gooi dan gewoon de compiler weg en ga helemaal over op een dynamische, niet-getypeerde taal. Groovy ofzo. Aan je compiler heb je toch niks meer, want je gaat je problemen run-time proberen op te lossen.
Java is een statisch getypeerde taal. Dat leidt niet altijd tot korte, krachtige programma’s, maar kan wel – als je het goed doet – in programma’s resulteren die werken simpelweg omdat ze compileren. Kortom: gebruik de compiler. Probeer ‘m niet over te slaan.
Reflectie is zeker nodig, maar moet je laatste strohalm zijn. Bijna altijd is er een betere oplossing. Bij de laatste twee gevallen die ik zag – op deze blog – is dat zeker het geval! Ik heb vele gevallen van reflectie in code gezien, en in bijna alle gevallen is mijn conclusie dat men het verkeerde probleem aan het oplossen was.
Vincent
20 April 2009 om 9:58 am
Dan ben ik benieuwd hoe jij je Singletons testbaar houdt.
20 April 2009 om 8:44 pm
Heel simpel: ik maak ze niet.
Geen hond bouwt nog (enterprise) applicaties zonder gebruik te maken van een dependency injection framework. En die stellen je prima in staat om Singletons te definieren zonder dat je dat in de klasse vast moet leggen. Dus waarom zou je allerlei technische trucs gaan toepassen? Helemaal als die het je onmogelijk maken je code goed te unit testen?
Singleton is een Design Pattern. Een pattern waar op zich niks mis mee is. Met veel van de technische uitwerkingen ervan wel.
22 April 2009 om 12:24 pm
Je bent dus tegen reflection, maar een fan van IoC? IoC frameworks doen ook alles @runtime, waardoor je compile time checks kwijtraakt.
22 April 2009 om 2:33 pm
Er is nog wel een nadeel van Singletons, Met de classloader is het mogelijk om meerdere instanties van je singleton te maken.
Worst case: Singleton instance per class loader. Zie ook issues met bijvoorbeeld Log4J en meerdere apps in een single JVM.
22 April 2009 om 9:39 pm
Jan-Kees, die opmerking van je is te provocerend en te (waarschijnlijk met opzet) te bekrompen om serieus op te reageren.
Appels en peren.
En ook nog eens niet waar, op meer dan 1 manier…
Oh ja, en Spring is niet het enige IOC framework.
24 April 2009 om 11:24 am
Ik meende juist op een bekrompen opmerking te reageren, maar goed, om een welles nietes discussie te voorkomen, hierbij een iets genuanceerder antwoord.
Mijn punt is dat Singletons bepaalde nadelen hebben, zoals testbaarheid. Ik draag daar een oplossing voor aan. Die oplossing heeft voor- en nadelen, zoals ook aangegeven in het stukje. Eén van die nadelen is inderdaad dat reflection meer bug gevoelig is dan alles wat je @compile-time kunt checken. Onderaan het stukje noem ik ook een alternatief (zie 2de bullet) dat geen reflection gebruikt.
De reden dat ik het stukje plaatste, is dat ik in de praktijk wel degelijk ontestbare Singletons tegenkom. Voor die gevallen is dit een relatief nette oplossing, maar het blijft een middel. Een middel met voor- en nadelen.
IoC is leuk, maar niet @compile-time (dat geldt niet alleen voor Spring). Dat is aan de ene kant een sterk punt van IoC, maar je verliest er een stukje veiligheid mee. Vandaar dat ik jouw opmerking over IoC zo vreemd vond. 3 regels reflection op een gecontroleerde plaats vind je slecht, maar een dependency @runtime opzoeken en injecteren vind je wel goed? Dat vind ik appels met peren vergelijken. En zeg nou zelf, hoeveel type safety raak je nou feitelijk kwijt? Zeker aangezien vrijwel alles in de reflection API generic is.
Maar goed, laten we het maar op “ruis op de lijn” houden. Volgens mij zijn we het er allebei over eens dat IoC in bepaalde gevallen nuttig is, reflection in bepaalde gevallen nuttig is en singletons in bepaalde gevallen nuttig zijn. Welk geval, daar heb ik in dit blog niets over gezegd, want dat is context afhankelijk.
Zijn we het nu eens?
24 April 2009 om 2:07 pm
Laten we teruggaan naar het oospronkelijk ‘probleem’. Ik snap namelijk het ‘niet tesbaar zijn’ van de deze singleton niet goed. Dat ligt volgens mij meer aan het feit dat
deze singleton op geen enkele manier benaderbaar is dan aan het feit dat het een singleton is. Een buitenstaander heeft dus ook niks aan deze singleton. Er zal op zijn minst een getter of een andere accessor methode moeten zijn om er iets mee te kunne. Als je simpelweg een getter toevoegt, kun je de BadSingletonClass prima testen :
public class BadSingletonTest {
@Before
public void setup() {
System.setProperty(“prop1″, “test value1″);
System.setProperty(“prop2″, “test value2″);
}
@Test
public void testSingleton() throws Exception {
BadSingleton obj1 = BadSingleton.getInstance();
BadSingleton obj2 = BadSingleton.getInstance();
Assert.assertTrue(“not same object” ,obj1==obj2);
}
@Test
public void testProperty() throws Exception {
BadSingleton obj = BadSingleton.getInstance();
Assert.assertEquals(obj.getProp1(), “test value1″);
Assert.assertEquals(obj.getProp2(), “test value2″);
}
}
26 April 2009 om 11:21 am
Laten we het maar op ruis op de lijn houden ja
Wat ik vooral bedoelde is dat bijna alle (Enterprise?) projecten een IoC container gebruiken. En als je die al gebruikt, waarom zou je dan nog met reflectie gaan goochelen? Immers, alle IoC containers zullen onder water ook reflectie gebruiken om hun doel te bereiken. Het zit er al in. En goed getest ook. Dus waarom zelf opnieuw implementeren? Dat wil dus niet zeggen dat ik een fan van IoC ben!
Als je echter nog geen IoC container gebruikt, dan vind ik het een ander verhaal.
IoC is natuurlijk ook run-time (waarom eigenlijk?), maar bijvoorbeeld met Google Guice kun je de bindings van interfaces aan implementatie wel gewoon compile-time en type-safe definiëren in een Module. Het lukt je niet een implementatie te koppelen aan een interface die de concrete klasse niet implementeert. Dat compileert gewoon niet. De enige echte fout die je nog kunt maken is dat je vergeet een implementatie te koppelen aan een implementatie en daar kom je dan pas run-time achter. (Ook al heb je een simpele integratietest die niks anders doet dan je Module in de lucht hijsen, dan nog is het te laat.)
Kom ik terug op mijn vraag: waarom is IoC eigenlijk altijd run-time?
26 April 2009 om 2:48 pm
“De enige echte fout die je nog kunt maken is dat je vergeet een implementatie te koppelen aan een implementatie en daar kom je dan pas run-time achter.”
Dat is dus precies hetzelfde als mijn 3 regeltjes reflection code. De enige fout die je kunt maken is dat je iets verandert aan de GoodSingleton constructor signature.
“Kom ik terug op mijn vraag: waarom is IoC eigenlijk altijd run-time?”
Stel dat je 2 implementaties hebt, 1 mock en 1 echte. De mock implementatie heeft de default constructor en de productie implementatie bevat een constructor met parameters, maar zonder @Inject. Dat gaat tijdens het draaien van de test goed en kan toch op de server fout gaan. Hence, runtime controle. Is dat erg? Ik denk van niet, net zoals dat het niet zo erg is dat mijn Singleton pas @runtime zal crashen bij een fout.
In een Guice app vinden wel meer checks @runtime plaats. Je kunt dus een access modifier verkeerd zetten (class/constructor/setter). Je kunt @Inject vergeten op een constructor of een empty constructor vergeten. Je kunt de bind() vergeten. Je kunt vergeten een implementatie te schrijven… Allemaal fouten die je kunt maken die de compiler niet detecteert. Het zijn deels theoretische gevallen, dat snap ik ook wel, maar in mijn optiek zijn bij mijn reflection Singleton de nadelen ook voornamelijk theoretisch.
Ik zie qua type safety dus vrij weinig verschillen tussen Guice en de Singleton. De Reflection API is generic en in dit geval type safe. Het enige waar je voor op moet letten qua type safety is dat je de signature van de GoodSingleton constructor niet moet veranderen, maar met 1 unit test (op getInstance) vang je zulke problemen waterdicht af. Thread safety is gemakkelijk aan te tonen (het zijn immers maar 2 kleine klassen) op basis van code analyse, terwijl je bij een framework maar moet hopen dat alles bugvrij is. Je haalt immers een enorme codebase binnen (snelle scan met Winrar levert in Guice 470 classes op).
Maar goed, het blijft een afweging, dus: Heb je geen IoC container? Wil je wel een Singleton? Moet je Singleton concurrent zijn (lijkt me een no-brainer)? Staat je SecurityManager setAccessible() toe? Wil je je Singleton unit testen (lijkt me ook een no-brainer)? Ben je bereid om een heel klein beetje type safety op te geven ten behoeve van testbaarheid? Wil je je code kunnen analyseren en niet meer dan 2 classes lezen?
Als je al deze vragen met JA kunt beantwoorden, dan is dit volgens mij een prima oplossing. Anders moet je waarschijnlijk tussen een standaard Singleton of IoC kiezen. Of geen Singleton gebruiken, want in veel gevallen heb je hem niet nodig.
Ps. Ik ben benieuwd of er voor Guice al APT Processors en Eclipse builders zijn. Lijken me nuttige tools. Dan kunnen ze @compile-time alle classes inspecteren op basis van de relaties tussen de classes/annotations en onderlinge consistentie controleren. @ImplementedBy is bijvoorbeeld erg eenvoudig te checken met APT, maar het is onmogelijk met standaard javac.
@Pieter: Idd. Bij reloaden van die applicaties wordt het nog vervelender, zeker als de client applicaties de referenties bewaren i.p.v. opvragen met getInstance(). Krijg je allemaal erg obscure bugs van (NullPointers, inconsistente data…).
27 April 2009 om 7:11 pm
Het grote voordeel van het gebruik van IoC frameworks ten opzichte van zelfgemaakte Singletons lijkt me meer op het gebied van toegang. Je moet erg vreemde dingen doen in een goed framework om dingen op de verkeerde manier te doen. Natuurlijk kan je ook in bijvoorbeeld Spring dingen doen die niet goed zijn, maar als je een normale gebruiker een handleiding geeft zal het meestal wel goed gaan. Het framework zelf duwt je namelijk de juiste kant op.
Zeker met Dependency Injection is dit het geval, want in de klasse die gebruik maakt van een dependency heb je alleen een constructor of setter, meer doe je daar niet.
Tevens denk ik dat je heel snel problemen krijgt als je op verschillende projecten zelfgemaakte oplossingen tegenkomt, het is naar mijn mening voor de meeste programmeurs erg goed om een vaste manier te hebben van werken.. en tegenwoordig lijkt Spring de defacto te zijn.
Er zitten natuurlijk altijd ‘nadelen’ aan het gebruik van een IoC framework, maarja… waar praten we dan over, deze dingen hoor je veel:
Guice (en Spring tegenwoordig): Je hebt annotations van je IoC framework in je code staan, dus zelf een dependency op het IoC framework. Je gecompileerde code heeft dus altijd het IoC framework nodig om te kunnen draaien. (Ja en? waarschijnlijk ook nog wel 10 andere frameworks)
Spring: De XML heeft een relatie met de Java-code, dus het hernoemen van een klasse kan ervoor zorgen dat het run-time niet meer werkt als je vergeet goed de XML-verwijzingen te veranderen. Maar een beetje IDE kan dit ook voor je oplossen natuurlijk.
Persoonlijk zou ik graag een keer een groot project met Guice willen doen, de echte “Java manier” met annotations. Want naast de snelheid (volgens sommige blogs 10x zo snel als Spring) heeft het nog een ander voordeel, de annotations zijn ook zelf-documenterend en makkelijker de refactoren.
@JKVA: Ja, deze post is compleet off-topic, maar ik vind het zelf schrijven van Singletons in de huidige enterprise wereld overboden en zonde van de tijd
28 April 2009 om 12:53 pm
Trouwens, je kan met Constructor Injection en het keyword final erg veel afdwingen met bijvoorbeeld je IoC framework. Als je in zo’n klasse zit weet je zeker dat:
- Degene die deze klasse heeft aangemaakt heeft ook de dependency gezet
- De dependency kan nooit meer gewijzigd worden
Bijvoorbeeld:
public class PersoonService {
//Compiler dwingt dat deze gevuld moet worden door een constructor
final PersoonDao persoonDao;
public PersoonService(final PersoonDao persoonDao) {
this.persoonDao = persoonDao; //Kan nooit meer gemuteerd worden!
}
}
Je kan dan zelfs een eigen (hardcoded) booster schrijven die de dependencies eenmalig zet, de instanties zijn dan misschien wel geen singletons, maar de dependencies zijn wel final en gezet.
Bijvoorbeeld:
public class Booster {
public void onStartup() {
//Wire everything
PersoonDao persoonDao = new PersoonDao(myJDBCConnectie);
PersoonService persoonService = new PersoonService(persoonDao);
PersoonBean bean = new PersoonBean(persoonService);
//etc etc
misschien met factory erbij
}
}
Yay, weer een eigen IoC framework geschreven, nog even een annotation-processor toevoegen, klaar