6
\$\begingroup\$

I have an Android app with three different Activity classes. Each of the Activities uses and changes config and user-customization data stored in a wrapped SharedPreferences. The code could stand some refactoring, so I'm thinking that rather than have each Activity instantiate the SharedPreferences by calling PreferenceManager.getDefault... in their constructors, I can make a 'base' abstract Activity to serve as a parent class and then have each of the Activities subclass the abstract Activity and wrap the SharedPreferences in an inner class of the abstract Activity, like so:

public abstract class AbstractBaseActivity extends AppCompatActivity
{
    private static AppPrefs instance;

    private interface PrefKeys
    {
        String KEY_BOOL_SETUP =  "KEY_BOOL_SETUP";
        String KEY_STR_USER_EMAIL = "KEY_STR_USER_EMAIL";
        String KEY_LONG_LAST_EMAIL_SENT = "KEY_LONG_LAST_USE";
        //...remaining keys omitted...//
    }

public static class AppPrefs implements PrefKeys
{
  private SharedPreferences prefs;
  private String no_email;

        private AppPrefs(AbstractBaseActivity activity)
        {
            prefs = PreferenceManager.getDefaultSharedPreferences(activity);
            no_email = activity.getString(R.string.no_email);
        }

        public static AppPrefs getInstance(AbstractBaseActivity activity)
        {
            if(instance == null)
            {
                instance = new AppPrefs(activity);
            }
            return instance;
        }

        public String getUserEmail()
        {
          return prefs.getString(AppPrefs.KEY_STR_USER_EMAIL, no_email);
        }

        public boolean setUserEmail(String email)
        {
          return prefs.edit().putString(AppPrefs.KEY_STR_USER_EMAIL, email).commit();
        }

         //... getter/setter methods to modify the SharedPreferences ...//
    }
}

I like this approach because it simplifies getting the SharedPreferences. I just call AppPrefs.getInstance(this) from within the Activities or AppPrefs.getInstance(getActivity()) from within a Fragment. Currently, my SharedPreferences are wrapped in a custom Object -- AppPrefs -- which uses obviously named getters and setters to interact with my SharedPreference data without actually having to muck about with key-value pairs and remembering to call commit() when changing data, but it is a stand-alone class. It seems to make sense to make the AppPrefs class an inner class of the AbstractBaseActivity class, but I'd welcome any criticisms of the approach.

I'm also uncertain about how I'm dealing with getting the AppPrefs object -- is there a better way to implement instantiation?

What are the downsides I'm not seeing?

\$\endgroup\$
6
  • \$\begingroup\$ Since the question involves refactoring a top-level singleton into an inner class, should I just delete the question? \$\endgroup\$ Sep 6, 2015 at 19:43
  • 1
    \$\begingroup\$ It's not a huge problem. As long as questions do a good job of displaying and explaining the code in question, the rest is either fine, or it'll get edited. \$\endgroup\$
    – Kaz
    Sep 6, 2015 at 19:46
  • 2
    \$\begingroup\$ @user5292387 No. Your title should just describe what the code does, in brief. \$\endgroup\$ Sep 6, 2015 at 19:47
  • \$\begingroup\$ @EthanBierlein, that's what has me confused, my question is not about what my code does, but rather on the choice I'm making about how to refactor it. So I'm not exactly sure how to write a title that satisfies some requirement that can't be met. \$\endgroup\$ Sep 6, 2015 at 19:59
  • \$\begingroup\$ @user5292387 The question doesn't necessarily have to describe what your code does, but generally, the title of your question should describe what the code does. \$\endgroup\$ Sep 6, 2015 at 20:03

1 Answer 1

5
\$\begingroup\$

Public class, private Interface?

I'm pretty confused by this setup.
The meaning is always trying working towards the most highest object( this case interface) possible.
I'm thinking that you created this setup so having the keys as constants.
But there is nothing wrong to have those keys as private static final String in your class self.

Java singleton.

I can't repeat it enough but if you want a singleton, please think enum in stead of private constructor.
Your singleton isn't thread safe or Serializable in this case, while if you use enum it is, and it's even easier to created it.

Abstract class root of all evil?

While I do use abstract classes, I'm always thinking also about putting the abstract methods in an interface and use that interface in the constructor.
This has multiple advantages of the abstract class.
First of all, you can create the base class final so no unexpected behavior could be implemented later on.
Then the interface you can swap at runtime (don't forget a check on null) and your class behave different but still in the correct way.

As I don't see the whole picture here I can't say it's good or not in this case.
But still, think about it, see the advantages/disadvantages and make your conclusion if it's good in this case or not.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.