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

Also read this [post](https://stackoverflow.com/questions/70689/what-is-an-efficient-way-to-implement-a-singleton-pattern-in-java) for correct Singleton Implementation. In a multi-thread environment, you may have some troubles — Scary Wombat, Jun 22 '18 at 07:14

score 8 · Accepted Answer · answered Jun 22 '18 at 07:11

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.

azro · Answer 2 · 2018-06-22T07:55:39.140

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");
    } 

}

Or those statements should be moved in the constructor – AxelH Jun 22 '18 at 07:15 — AxelH, Jun 22 '18 at 07:15

JAVA singleton not working

2 Answers2