1

Does the class below implement the singleton pattern? I'm really confused.

class Customer {

    static private Map costumers = new HashMap();

    private Customer() {
    }

    public static Customer getInstance(String name) {
        Customer instance = (Customer) costumers.get(name);
        if (instance == null) {
            instance = new Customer();
            costumers.put(name, instance);
        }
        return instance;

    }
}

Thanks.

O. Joy
  • 21
  • 4

3 Answers3

12

No, it doesn't. Thread-safety and type-safety issues aside, I can very easily get two different instances of Customer:

Customer x = Customer.getInstance("first");
Customer y = Customer.getInstance("second");
System.out.println(x == y); // false

Therefore it's not a singleton. It's a sort of factory/flyweight pattern hybrid, but it's certainly not the singleton pattern.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 3
    Nothing personal, but I think the amount of upvotes on this testifies to the [Jon Skeet effect](https://meta.stackexchange.com/questions/41442/jon-skeet-effect-high-reputation-users-effect). – shmosel Mar 26 '17 at 09:00
1

Its not. In the given example, a new Customer instance is created for each different Customer name. Which should not happen in case of singleton. There sould be only one Customer instance created and shared for all the customer names. The Customer instance needs to be private static and it shoud be created and assigned only once.

OTM
  • 656
  • 5
  • 8
1
public class SingltonPattern {

private static SingltonPattern instance = null;

protected SingltonPattern() {}

public static SingltonPattern getInstance() {
        if (instance == null) {
                // Thread Safe - Second layer
                synchronized (SingltonPattern.class) {
                        if (instance == null) {
                                instance = new SingltonPattern();
                        }
                }
        }
        return instance;
}

Here is a pattern for Singlton DP.

ReDevil
  • 77
  • 6
  • There's rarely a need for double-checked locking though - there are simpler ways of implementing a thread-safe singleton in Java. This also doesn't address the OP's code at all. – Jon Skeet Mar 26 '17 at 09:04
  • @JonSkeet There's rarely a need for double-checked locking though - totally disagree (we can discuss it but it is off-topic here). there are simpler ways of implementing a thread-safe singleton in Java - correct just gave one example. This also doesn't address the OP's code at all - I Just gave an abstract example so OP can figure up what is wrong with his. – ReDevil Mar 26 '17 at 10:51
  • 1
    Simpler ways of implementing: single-element enum, or just initialize the `instance` variable directly. It's very rare that you need this more complicated and error-prone pattern. (Even if you want laziness with the ability to call other static methods without initializing, a nested class is a simpler approach.) And if you're not addressing the OP's question, it doesn't belong as an answer to the question, IMO. – Jon Skeet Mar 26 '17 at 11:28
  • 1
    Actually, having seen you've got a protected constructor, this isn't even a valid implementation of the singleton pattern... – Jon Skeet Mar 26 '17 at 11:51