2

Currently, I have a class whose constructor takes a username, password, and Context. I would like to be able to access this object from anywhere, so I was thinking of implementing a singleton pattern.

The current constructor uses the credentials passed in to authenticate future api calls through that class. If I were to implement the singleton pattern, my first thought would to be have the getInstace() method take a username, password, etc.., but it seems wrong to have to pass that info everytime i grab an instance. Because of this I was thinking of adding some sort of .authenticate(usr, pswrd) method to be called when grabbing the first instance.

My question is, is this the correct approach? If not, what would be a good way to handle this? Heres the current code:

constructor:

public Play(String username, String password, Context context) {
        api = getApi(username, password);
        Intent intent = new Intent(context, MyService.class);
        context.bindService(intent, mConnection, Context.BIND_AUTO_CREATE);

        //check if first run
        //if so, call api for info and store locally
        //if not, update as needed

        SharedPreferences pref = context.getSharedPreferences("pref", Context.MODE_PRIVATE);
        SharedPreferences.Editor editor = pref.edit();

        if (pref.getBoolean("first_run", true)) {
            loadInitialData(context);
        }

        editor.putBoolean("first_run", false);
        editor.commit();
    }
Orbit
  • 2,985
  • 9
  • 49
  • 106
  • Why not construct it once, and then pass the instance of `Play` to the places you need it? Singletons - especially ones which need constructor parameters - are problematic. – Andy Turner May 23 '16 at 17:25
  • @AndyTurner Constructing once and passing the instance-thats the singleton pattern – Orbit May 23 '16 at 17:26
  • no, it's not. A singleton is something of which there can only be one instance; you can create just one instance of something without it being a singleton. – Andy Turner May 23 '16 at 17:28
  • Sure, just some of the places I need it are fairly "removed" from the rest of the code I guess. Im not sure how I would get it in there. – Orbit May 23 '16 at 17:29
  • Well, that's another matter. But there are [many reasons why singletons are bad](http://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons). – Andy Turner May 23 '16 at 17:30
  • without caring for another singleton debate: @Orbit I would think about authenticating user credentials outside of my singleton and storing the data upon a successful validation. `Play.getInstance().setUser(...)` – zec May 23 '16 at 17:45
  • @AndyTurner, I did check the link mentioned by you and I am convinced that Singleton is a bad idea unless something has to be global which is required by the application. Could you shade some light on the topic how to come around the issue, when we require global variables without using Singleton Pattern – Pankaj Nimgade May 23 '16 at 19:58

1 Answers1

1

Singleton pattern restricts the instantiation of a class and ensures that only one instance of the class exists in the java virtual machine. The singleton class must provide a global access point to get the instance of the class. Singleton pattern is used for logging, drivers objects, caching and thread pool

This code is not tested but should give you an idea how you can use singleton pattern while using SharedPrefrencess.

Constructor is private, So only getInstance() method can access the instance, so you will create an instance of this class if it doesn't exists or if instantiated previously use that instance

synchronization is required to make sure when multiple thread are trying to make a instance for the first time

import android.content.Context;
import android.content.SharedPreferences;

/**
 * Created by Pankaj Nimgade on 23-05-2016.
 */
public class Play {

    /**
     * volatile keyword ensures that multiple threads handle the uniqueInstance
     * variable correctly when it is being initialized to Singleton instance
     */
    private volatile static Play play;

    private static final String XML_FILE = "play_xml_file.xml";
    private static final String KEY_DATA = "SOME_DATA_KEY";
    private static final String KEY_USERNAME = "SOME_USERNAME_KEY";
    private static final String KEY_PASSWORD = "SOME_PASSWORD_KEY";

    private static SharedPreferences sharedPreferences;

    private static SharedPreferences.Editor editor;

    private Play() {
    }

    public static Play getInstance(Context context) {
        if (play == null) {
            synchronized (Play.class) {
                if (play == null) {
                    sharedPreferences = context.getSharedPreferences(XML_FILE, Context.MODE_PRIVATE);
                    editor = sharedPreferences.edit();
                    play = new Play();
                }
            }
        }
        return play;
    }

    public boolean saveSomeData(String someData) {
        editor.putString(KEY_DATA, someData);
        return editor.commit();
    }

    public String readSomeData() {
        return sharedPreferences.getString(KEY_DATA, "default Value");
    }

    public boolean saveUserNameData(String username) {
        editor.putString(KEY_USERNAME, username);
        return editor.commit();
    }

    public String readUserNameData() {
        return sharedPreferences.getString(KEY_USERNAME, "default username Value");
    }

    public boolean savePasswordData(String password) {
        editor.putString(KEY_PASSWORD, password);
        return editor.commit();
    }

    public String readPasswordData() {
        return sharedPreferences.getString(KEY_PASSWORD, "default password value");
    }
}

in this above approach I am making instance creation of the class lazy, as the instance will only be created if demanded, although the code is thread safe and will work on all Java version you may want to consider different approach to implement this if you are using Java 5 and above.

https://sourcemaking.com/design_patterns/singleton/java/1

public class Singleton {
  // Private constructor prevents instantiation from other classes
  private Singleton() {}

  /**
   * SingletonHolder is loaded on the first execution of Singleton.getInstance() 
   * or the first access to SingletonHolder.INSTANCE, not before.
   */
  private static class SingletonHolder { 
    private static final Singleton INSTANCE = new Singleton();
  }

  public static Singleton getInstance() {
    return SingletonHolder.INSTANCE;
  }
}

The inner class is referenced no earlier (and therefore loaded no earlier by the class loader) than the moment that getInstance() is called. Thus, this solution is thread-safe without requiring special language constructs (i.e. volatile or synchronized).

Pankaj Nimgade
  • 4,529
  • 3
  • 20
  • 30