Singleton Controller Java

Question

I was reading about singletons the other day and thought to implement them in a project of mine, but I didn't like the way it seemed to flow logically, so I created what I'd call a controller class to manage the state of the singleton object. I want to make sure that the logic checks out though, and that I'm not inadvertently spawning off additional instances.

//Controller for SaveSystem, allows the class to be created once for the 
//duration of the application at the global level (believe this is called a 
//singleton pattern)

public class SaveSystemContr {
  private static SaveSystem saveSystemInstance;
  public SaveSystem GetSaveSystemData() {
    return saveSystemInstance;
  }

  public void SetSaveSystem(SaveSystem _saveSystem) {
    if(saveSystemInstance!=null) {
        saveSystemInstance=_saveSystem;
    }
  }

  public static SaveSystem getSaveSystemInstance(final FirebaseAuth _auth, final LoadProjFragment _LFP) {
    if(saveSystemInstance==null) {
        saveSystemInstance = new SaveSystem(_auth, _LFP);
    }

    return saveSystemInstance;
  }

  public SaveSystemContr() {} //THE WAY IS SHUT!
}

Edit* I do not view this as a duplicate of the question referenced, as that was a typical/standard implementation of a singleton, and this uses a different model altogether by utilizing a controller to manage the state of the singleton.

Answer 1

I want to make sure that the logic checks out though, and that I'm not inadvertently spawning off additional instances.

Looks like you can create as many instances as you want:

SaveSystemContr controller = new SaveSystemContr();

// Create instance 1
controller.SetSaveSystem(new SaveSystem(auth, lfp));

// Create instance 2
controller.SetSaveSystem(new SaveSystem(auth, lfp));

// ...

Why do you have a setter method if you only want 1 instance?

A very basic singleton will look like this:

public final class SaveSystemSingleton {
  // This class should only be accessed statically. Don't allow instance creation
  private SaveSystemSingelton() {}

  private static SaveSystem saveSystem;

  // Encapsulate creation logic. Passing in params is misleading because
  // all subsequent calls will do nothing with the params.
  public static SaveSystem get() {
    // If accessing from multiple threads do Double Check Locking instead!
    if (saveSystem == null) {
      saveSystem = new SaveSystem(new FirebaseAuth(), new LoadProjFragment());
    }
    return saveSystem;
  }
}

If you really need to pass in the params, define a separate static setter method, and throw an exception if it is called more than once or if get() is called without first calling the setter.

Also, you should check out dependency injection (e.g. Dagger2), which makes creating instances of objects and scoping them clear and easy.

Answer 2

Make the constructor private and remove getters and setters :

    //Allows the OBJECT (not class) to be created once only    
    public class SaveSystem {

      private static SaveSystem saveSystemInstance;

      //make constructor private 
      private SaveSystem(final FirebaseAuth _auth, final LoadProjFragment _LFP) {
        //todo complete constructor
      }

      public static SaveSystem getSaveSystemInstance(final FirebaseAuth _auth, final LoadProjFragment _LFP) {
        if(saveSystemInstance==null) {
            saveSystemInstance = new SaveSystem(_auth, _LFP);
        }   
        return saveSystemInstance;
      } 
    }

As you see it can be applied on SaveSystem without a controller.