JAVA singleton not working

Question

I enhanced/tested the coding I found on ArrayList initialized/accessed using Singleton class

import java.util.ArrayList;

public class SingletonArrayList {

    private static SingletonArrayList mInstance;
    private static ArrayList<String> list = null;

    public static SingletonArrayList getInstance() {
        if (mInstance == null)
            mInstance = new SingletonArrayList();
        SingletonArrayList.list.add("a");
        SingletonArrayList.list.add("b");
        SingletonArrayList.list.add("c");
        return mInstance;
    }

    private SingletonArrayList() {
        list = new ArrayList<String>();
    }

    // retrieve array from anywhere
    public ArrayList<String> getArray() {
        return SingletonArrayList.list;
    }

}

Then I made a testclass where I call the above singleton two times:

import java.util.ArrayList;

public class TestSingletonArrayList {

    public static void main(String[] args) {


        ArrayList<String> array = SingletonArrayList.getInstance().getArray();
        for (int i = 0; i < array.size(); i++) {
            System.out.println(array.get(i));
        }
        System.out.println("-----------");
        ArrayList<String> array2 = SingletonArrayList.getInstance().getArray();
        for (int i = 0; i < array2.size(); i++) {
            System.out.println(array2.get(i));
        }

    }

}

The output is:

a
b
c
-----------
a
b
c
a
b
c

This seems very strange. I expected that the second call of the singleton class will only return a,b,c and NOT a,b,c,a,b,c

What is wrong? I expected only a,b,c as it is a singleton

Thanks, regards Mario

Answer 1

Change it like so,

public class SingletonArrayList {
    private static SingletonArrayList mInstance;
    private static ArrayList<String> list = null;

    public static SingletonArrayList getInstance() {
        if (mInstance == null) {
            mInstance = new SingletonArrayList();
            SingletonArrayList.list.add("a");
            SingletonArrayList.list.add("b");
            SingletonArrayList.list.add("c");
        }

        return mInstance;
    }

    private SingletonArrayList() {
        list = new ArrayList<String>();
    }

    // retrieve array from anywhere
    public ArrayList<String> getArray() {
        return SingletonArrayList.list;
    }

}

The issue here is you have to create the object and initialize it both at once. In your solution you create it once, and for each invocation you add items separately. Still it has only one instance, but you keep adding elements to that singleton instance for each invocation of getInstance, which is counter intuitive.

Answer 2

with your code, each time you call you singleton instance (because only the next instruction refers to the if), you're adding the three elements a, b, and c, so at first call you have abc, and at second call you have abcabc

You need to add them only when you create the instance :

public static SingletonArrayList getInstance() {
    if (mInstance == null){
        mInstance = new SingletonArrayList();
        SingletonArrayList.list.add("a");
        SingletonArrayList.list.add("b");
        SingletonArrayList.list.add("c");
    }
    return mInstance;
}

---

Also, the real Singleton pattern is with a final class and a final instance to disallow to be modified, yours is good for a lazy initialisation but you modify it, so the real is :

• final class
• private constructor
• static final instance

public final class SingletonArrayList {

    private static final SingletonArrayList mInstance = new SingletonArrayList();
    private static ArrayList<String> list = null;

    public static SingletonArrayList getInstance() {
        return mInstance;
    }

    private SingletonArrayList() {
        list = new ArrayList<String>();
        SingletonArrayList.list.add("a");
        SingletonArrayList.list.add("b");
        SingletonArrayList.list.add("c");
    } 

}