3

I have been reading recently that Singletons are often a design pattern that are abused. I understand that globals can be bad and that singletons are not extendable (thus not good OO design), but I was wondering if in this instance I am using it 'correctly'.

Essentially, I have some software running on a - lets say - vending machine. It has an administrator console where you can login and configure some stuff. For example, today I want a coke bottle to cost $3 and a sprite to cost $2, so I can just log in to this admin console and edit these values. Then, in my code, I can read these configuration parameters from the console using some java code:

AdminConsole adminConsole = new AdminConsole();
Map<String, Int> drinkPriceMap = adminConsole.getSettings();

This loads a map of key value pairs of things like Coke:3, Sprite:2, Water:2 and will happen when the 'main' class is first instantiated on machine-bootup.

The prices of the drinks should only be updated per hardware reset.

Now, because the hardware of this vending machine is very limited, and pulling the settings from the console is quite memory-heavy for it, I want to do things with this in mind.

When the AdminConsole is instantiated it pulls these values from the admin console and generates the map. The act of pulling these values is a memory expensive operation, whereas the map is not such a big deal. What I want to do in the code is therefore make the AdminConsole a singleton so it can only be instantiated once. This is so that when a new developer joins the team and is unaware that it is a memory-expensive operation on a memory-limited hardware, he will not accidentally instantiate it multiple times.

Instead, it would look something like this:

AdminConsole adminConsole = AdminConsole.getInstance();
Map<String, Int> drinkPriceMap = adminConsole.getSettings();

So, do you think think I am using the singleton in a good manner here, or do you think there is a better way I could go about doing this?

DeaIss
  • 2,525
  • 7
  • 27
  • 37

2 Answers2

2

Using a single instance of of class is not what makes the singleton pattern problematic. Being unable to extend it is not problematic either. What is problematic is the way this single instance is obtained:

  • it couples every class using the singleton to this concrete class
  • it makes it basically impossible to unit test every class using the singleton, because there is no way to replace this unique Singleton instance by a mock instance.

A much better way to achieve what you want is to pass the singleton to the objects which need it. That is called dependency injection:

public class AdminConsole implements PriceProvider {
    @Override
    public Map<String, Integer> getPriceMap() {
        ...
    }
    ...
 }

public class SomeClassWhichNeedsThePrices {
    private PriceProvider priceProvider;

    public SomeClassWhichNeedsThePrices(PriceProvider priceProvider) {
        this.priceProvider = priceProvider;
    }
}

...

public static void main(String[] args) {
    AdminConsole console = new AdminConsole();
    console.initialize(); // read the prices once 
    SomeClassWhichNeedsThePrice sc = new SomeClassWhichNeedsThePrice(console);
    ...
}

Now, to unit-test the class SomeClassWhichNeedsThePrice, you can simply pass it a fake PriceProvider, without needing a real AdminConsole at all:

PriceProvider fakePriceProvider = new PriceProvider() {
    @Override
    public Map<String, Integer> getPriceMap() {
        Map<String, Integer> fakePrices = new HashMap<>();
        fakePrices.put("Coke", 2);
        return fakePrices;
    }
};
// you could also use a mocking framework like Mockito

SomeClassWhichNeedsThePrice sc = new SomeClassWhichNeedsThePrice(fakePriceProvider);
assertThat(sc.computePriceForThreeCokes()).toBe(6);
JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
  • Is there a reason why you didn't made AdminConsole as a singleton in this case? New developers can still make multiple instances of AdminConsole. Right? – filip Aug 23 '14 at 12:43
  • It is a singleton: only one instance exists in the program. It does not implement the singleton pattern (`getInstance()` method) because it's precisely what I want to avoid. – JB Nizet Aug 23 '14 at 12:46
  • A couple of things. 1. What is to stop someone writing AdminConsole console = new AdminConsole() and then calling initialize in other parts of the codebase, potentially causing the machine to slow down and lose hundreds of pounds? 2. Unfortunately, I do not have access to the main method, only the class that is instantiated by a complex/unmodifiable framework and needs to use the admin console. 3. Unit testing is not a big issue - I only need to mock the admin console settings once and I can hit all lines of code and test it. Also, I am already using powermock with mockito which can test it. – DeaIss Aug 23 '14 at 13:19
  • 1. package-protected access, code reviews, documentation. The use of a dependency injection framework discourages it. It's much simpler to use `@Inject PriceProvider priceProvider` to get an instance of PriceProvider than to find which class implements it, create it, initialize it, etc. Powermock is a slow hack to test legacy code that hasn't been written with testability in mind. – JB Nizet Aug 23 '14 at 14:32
  • Would love to use a dependency injection framework, unfortunately when you work at a big company you do not get to use frameworks and technologies you want! I like the idea of package protection, but still it does not stop someone instantiating the class within that package. I can javadoc as much as I want and do code reviews, but when I leave the company and someone new comes along and doesn't read the javadoc (happens a lot), its going to cost the company. This can all be avoided by using a singleton as far as I can tell, which can be unit tested just fine. – DeaIss Aug 23 '14 at 15:17
2

I looked up the Singleton pattern discussion on Stack Overflow and while it looks to me that 99.9% of people think that Singleton is the root of all evil in the world I stumbled on a post that may help you. If you read the entire discussion you will see that everyone has their own thoughts about singletons with main keywords (fancy programmer words) like unit testing, dependency injection, inversion of control, etc.

Community
  • 1
  • 1
filip
  • 414
  • 3
  • 14