2

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.

bcorso
  • 45,608
  • 10
  • 63
  • 75
Adam R. Turner
  • 139
  • 1
  • 2
  • 14
  • It's not supposed to be a singleton pattern directly, rather a controlling class that manages a single instance of another class. It seemed to make more sense to me at the time, but maybe it's unnecessary/problematic. – Adam R. Turner Feb 03 '18 at 03:56
  • Possible duplicate of [Singleton class in java](https://stackoverflow.com/questions/2111768/singleton-class-in-java) – Shubhendu Pramanik Feb 03 '18 at 03:59
  • that appears to a typical singleton class, yes, but this one I implement as: 'SaveSystemContr SSC = new SaveSystemContr();' 'saveSystem = SSC.getSaveSystemInstance(auth, this);' – Adam R. Turner Feb 03 '18 at 04:04
  • 2
    My question is: what problem are you trying to solve? Design patterns exist to solve specific design problems. What problem is this design solving? – Vince Feb 03 '18 at 04:11
  • 1
    If you're considering this kind of thing, it is not that far from having a Dependency injection framework manage your objects' scopes, DI lets you have singletons without the boilerplate of the controller and the ugliness of static references. – Nathan Hughes Feb 03 '18 at 05:28
  • Good point Vince. I think I was primarily concerned with maintainability, it seemed like having this part of the code under my SaveSystem class would clutter it up. I tried to build that class as efficient as possible because it's complexity grows substantially because it deals with waterfall-esque queries (another term here...). – Adam R. Turner Feb 03 '18 at 06:22

2 Answers2

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.

bcorso
  • 45,608
  • 10
  • 63
  • 75
  • This seems to be what I was after. I like how it has an external class to manage an internal class. I do need to pass in the parameters, so I will be looking at the double locking mechanism you mentioned. – Adam R. Turner Feb 03 '18 at 06:26
1

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.

c0der
  • 18,467
  • 6
  • 33
  • 65