CODEWITHSUNDEEP
javasingleton
Ishan Soni
Ishan Soni

Reputation: 273

Implementing Singleton using factory method

I am trying to create a class that has a singleton property using a static factory method..

package ishan.Beans;

public class ControlManager {

    private static ControlManager controlManager=null;

    private  double id;

    private ControlManager()
    {
        this.id=Math.random();
    }

    public static ControlManager getControlManager()
    {

        if(null==controlManager)
            return new ControlManager();

        return controlManager;
    }

    public double getId() {
        return id;
    }

}


package ishan.Beans;

public class Usage {

    public static void main(String a[])
    {
        ControlManager cManager=ControlManager.getControlManager();

        ControlManager c=ControlManager.getControlManager();

        System.out.println(c);
        System.out.println(cManager);
    }

}

Every time I run this code, I get different instances of ControlManager in c and cManager. I am unable to figure out the problem or what is it that I am doing wrong.

Upvotes: 3

Views: 101

Answers (3)

Dhanushka Gayashan
Dhanushka Gayashan

Reputation: 724

Your code should be change as:​

public static ControlManager getControlManager() {
    if(controlManager == null)controlManager = new ControlManager();          
    return controlManager;
}

Good Luck !!!

Upvotes: 0

ifloop
ifloop

Reputation: 8386

Change your getControlManager():

From

return new ControlManager();

to

controlManager = new ControlManager();

This is, what it should look like in the end:

public static ControlManager getControlManager() {
    if(controlManager == null) {
        controlManager = new ControlManager();
    }

    return controlManager;
}


In addition something opinion based, but backed up by conventions imo:
When writing conditional statements (if) the order of your conditions should stick to the way of naturally speaking the statement. Example:

  • Bad: if (null == contorlManager)
  • Good: if (controlManager == null)

The reason or goal behind this is, to keep your code readable. No one asks: "Is green the traffic light?".

For further information on this, see Yoda Conditions

Upvotes: 0

rolfl
rolfl

Reputation: 17707

You are not saving away the new instance you create... your code is:

public static ControlManager getControlManager()
{

    if(null==controlManager)
        return new ControlManager();

but should be:

public static ControlManager getControlManager() {

    if(controlManager == null) {
        controlManager = new ControlManager();
        return controlManager;
    }

Upvotes: 1

Related Questions