Is this class a singleton?

Question

Is the class listed below a singleton? Since the constructor is declared as public, can I infer that the class is a singleton with wrong implementation?

public class CreateDevice extends Functionality{

  private static Simulator simulator;
  ConnectionDB connect = ConnectionDB.getInstance();


  public CreateDevice(Simulator simulator){
    this.simulator = simulator;
  }

  private static CreateDevice instance;
  synchronized public static CreateDevice getInstance() {
    if(instance == null){
      instance = new CreateDevice(simulator);
    }       
      return instance;
  }
}

Answer 1

In a word - no. Since the constructor is public, anyone can create a new instance of it wherever and whenever they want. Making the constructor private should solve the issue, and turn the class into a singleton.

Answer 2

I would say you are correct in saying that this would not be a Singleton in the current implementation.

Either you need to make the constructor private (which is what is usually done), or if for legacy reasons you need a public constructor, you should create a constructor that checks if the instance is null, and either throw an exception if it already exists or create it by calling another private constructor, assign it to instance, and return it. The first option is preferable, althouth the other works fine for most projects where a refactor is too expensive.

Answer 3

You can make it a singleton, but then you need to find an other way to inject the simulator into it.

In the current implementation, the simulator is set the first time somebody calls the constructor. That constructor is very weird because it sets a static field. It will also immediately open a connection, separate from the connection that is used by the singleton instance.

If the getInstance() method is called before the constructor is called at least once, the simulator is never set.

To make it a proper singleton, you can remove the constructor and add a private no-args constructor. You'll also need a static setSimulator() method that sets the static field, and make sure it's called before any other interaction with the simulator is required.

If you have dependencies between singletons, I'd recommend to go for the Inversion-of-Control pattern, where an IoC container creates the service objects and wires them together.