Singleton implementation 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 ControlManager instances in c and cManager. I cannot figure out the problem or what I am doing wrong.

+3


source to share


3 answers


You are not saving 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;
    }

      

+1


source


Change your getControlManager()

:

FROM

return new ControlManager();

      

to

controlManager = new ControlManager();

      

This is what it should look like at the end:



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

    return controlManager;
}

      


Also, something opinion-based, but backed up by imo conventions:
When writing conditional operators ( if

), the order of your conditions should follow a natural expression. Example:

  • Poorly: if (null == contorlManager)

  • Good: if (controlManager == null)

The reason or purpose for this is to keep the code readable. Nobody asks, "Is the traffic light green?"

For more information on this see Yoda Terms

0


source


Your code should be changed like:

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

      

Good luck !!!

0


source







All Articles