Wrapper around default shared preferences in Android

Question

Retrieving an instance of (Default) SharedPreferences and then calling the six methods that may be called to put/get a value gets pretty old pretty quickly, so I wrote this:

import android.annotation.SuppressLint;
import android.annotation.TargetApi;
import android.content.Context;
import android.content.SharedPreferences;
import android.content.SharedPreferences.Editor;
import android.os.Build;
import android.preference.PreferenceManager;

import java.util.ArrayList;
import java.util.List;
import java.util.Set;

public final class AccessPreferences {

    private static final List<Class<?>> CLASSES = new ArrayList<Class<?>>();
    static {
        CLASSES.add(String.class);
        CLASSES.add(Boolean.class);
        CLASSES.add(Integer.class);
        CLASSES.add(Long.class);
        CLASSES.add(Float.class);
        CLASSES.add(Set.class);
    }

    private AccessPreferences() {}

    private static SharedPreferences prefs;

    // TODO: do I need synchronized ? (1)
    private static synchronized SharedPreferences getPrefs(Context ctx) {
        if (prefs == null) {
            prefs = PreferenceManager.getDefaultSharedPreferences(ctx);
        }
        return prefs;
    }

    @TargetApi(Build.VERSION_CODES.HONEYCOMB) // (5)
    public static <T> void persist(Context ctx, String key, T value) {
        @SuppressLint("CommitPrefEdits")
        final Editor ed = getPrefs(ctx).edit();
        if (value == null) {
            // commit it as that is exactly what the API does - can be retrieved
            // as anything but if you give get() a default non null value it
            // will give this default value back
            ed.putString(key, null);
        } else if (value instanceof String) ed.putString(key, (String) value);
        else if (value instanceof Boolean) ed.putBoolean(key, (Boolean) value);
        // TODO : IS THE ORDER OF FLOAT, INTEGER AND LONG CORRECT ?  // (4)
        else if (value instanceof Integer) ed.putInt(key, (Integer) value);
        else if (value instanceof Long) ed.putLong(key, (Long) value);
        else if (value instanceof Float) ed.putFloat(key, (Float) value);
        else if (value instanceof Set) {
            if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.HONEYCOMB) {
                throw new IllegalStateException(
                        "You can add sets in the preferences only after API "
                            + Build.VERSION_CODES.HONEYCOMB);
            } // (5)
            // The given set does not contain strings only --> TODO : not my
            // problem ? probably cause the one who filled it made the mistake (3)
            // Set<?> set = (Set<?>) value;
            // if (!set.isEmpty()) {
            // for (Object object : set) {
            // if (!(object instanceof String))
            // throw new IllegalArgumentException(
            // "The given set does not contain strings only");
            // }
            // }
            @SuppressWarnings({ "unchecked", "unused" })
            Editor soIcanAddSuppress = ed
                    .putStringSet(key, (Set<String>) value);
        } else throw new IllegalArgumentException("The given value : " + value
            + " cannot be persisted");
        ed.commit();
    }

    @SuppressWarnings("unchecked")
    @TargetApi(Build.VERSION_CODES.HONEYCOMB) // (5)
    public static <T> T retrieve(Context ctx, String key, T defaultValue) {
        // if the value provided as defaultValue is null I can't get its class
        if (defaultValue == null) {
            // if the key (which can very well be null btw) !exist I return null
            // which is both the default value provided and what Android would
            // do (as in return the default value) (TODO: test)
            if (!getPrefs(ctx).contains(key)) return null;
            // if the key does exist I get the value and..
            final Object value = getPrefs(ctx).getAll().get(key);
            // ..if null I return null
            if (value == null) return null;
            // ..if not null I get the class of the non null value. Problem is
            // that as far as the type system is concerned T is of the type the
            // variable that is to receive the default value is. So :
            // String s = AccessPreferences.retrieve(this, "key", null);
            // if the value stored in "key" is not a String (for instance
            // `"key" --> true` or `"key" --> 1.2`) a ClassCastException will
            // occur _in the assignment_ after retrieve returns
            // TODO : is it my problem ? This :  // (2)
            // SharedPreferences p =
            // PreferenceManager.getDefaultSharedPreferences(ctx);
            // int i = p.getInt(KEY_FOR_STRING, 7);
            // results in a class cast exception as well !
            final Class<?> clazz = value.getClass();
            // TODO : IS THE ORDER OF FLOAT, INTEGER AND LONG CORRECT ?
            for (Class<?> cls : CLASSES) {
                if (clazz.isAssignableFrom(cls)) {
                    try {
                        // I can't directly cast to T as value may be boolean
                        // for instance
                        return (T) clazz.cast(value);
                    } catch (ClassCastException e) { // won't work see :
                        // https://stackoverflow.com/questions
                        // /186917/how-do-i-catch-classcastexception
                        // basically the (T) clazz.cast(value); line is
                        // translated to (Object) clazz.cast(value); which won't
                        // fail ever - the CCE is thrown in the assignment (T t
                        // =) String s = AccessPreferences.retrieve(this, "key",
                        // null); which is compiled as
                        // (String)AccessPreferences.retrieve(this, "key",
                        // null); and retrieve returns an Integer for instance
                        String msg = "Value : " + value + " stored for key : "
                            + key
                            + " is not assignable to variable of given type.";
                        throw new IllegalStateException(msg, e);
                    }
                }
            }
            // that's really Illegal State I guess
            throw new IllegalStateException("Unknown class for value :\n\t"
                + value + "\nstored in preferences");
        } else if (defaultValue instanceof String) return (T) getPrefs(ctx)
                .getString(key, (String) defaultValue);
        else if (defaultValue instanceof Boolean) return (T) (Boolean) getPrefs(
            ctx).getBoolean(key, (Boolean) defaultValue);
        // TODO : IS THE ORDER OF FLOAT, INTEGER AND LONG CORRECT ?
        else if (defaultValue instanceof Integer) return (T) (Integer) getPrefs(
            ctx).getInt(key, (Integer) defaultValue);
        else if (defaultValue instanceof Long) return (T) (Long) getPrefs(ctx)
                .getLong(key, (Long) defaultValue);
        else if (defaultValue instanceof Float) return (T) (Float) getPrefs(ctx)
                .getFloat(key, (Float) defaultValue);
        else if (defaultValue instanceof Set) {
            if (android.os.Build.VERSION.SDK_INT < Build.VERSION_CODES.HONEYCOMB) {
                throw new IllegalStateException(
                        "You can add sets in the preferences only after API "
                            + Build.VERSION_CODES.HONEYCOMB);
            } // (5)
            // The given set does not contain strings only --> TODO : not my
            // problem ? probably cause the one who filled it made the mistake
            // Set<?> set = (Set<?>) defaultValue;
            // if (!set.isEmpty()) {
            // for (Object object : set) {
            // if (!(object instanceof String))
            // throw new IllegalArgumentException(
            // "The given set does not contain strings only");
            // }
            // }
            return (T) getPrefs(ctx).getStringSet(key,
                (Set<String>) defaultValue);
        } else throw new IllegalArgumentException(defaultValue
            + " cannot be persisted in SharedPreferences");
    }
}

My questions are:

1. Do I need to retrieve the preferences in a synchronized block (I understand that put/get operations are already synchronized)?
2. When I get a null as a default in the retrieve method, I am not sure I adhere to the generics' contract - there is the possibility of a ClassCastException - but so is there in the original framework. Could I get completely rid of the CLASSES casts and still be as typesafe as Android? See my comments in code.
3. generics' contract part II - if I get a Set as a parameter I believe I do not have to check if it is indeed a Set<String> - am I right?
4. The order I check (via instanceof and isAssignableFrom) what kind of number I got is : Integer, Long, Float. Is my order correct? - think of a call like persist(ctx, key, 349857094387593475). Would completely dropping Integer checks do the trick (and be acceptable)?
5. Do I handle the HONEYCOMB addition of Set<String> right (by throwing an exception if one calls my methods in pre HONEYCOMB build passing a Set in)?

Follow my TODOs in the code for the particular points I make.

Here is my current solution for anyone interested.

Part of this question is answered here.

Answer 1

I'm not too familiar with Android but I've found an issue with the code: both persist and retrieve methods contains the similar if-else chain. You should replace them with polymorphism:

• Refactoring: Improving the Design of Existing Code by Martin Fowler: Replacing the Conditional Logic on Price Code with Polymorphism
• Replace Conditional with Polymorphism

To answer some of your questions:

• (4) The order is not important since there isn't any class (in Java, at least) which instanceof more than one of Long, Integer and Float.

• (5) If it's invalid input data, I'd throw an exception with a detailed error message to the caller.

Answer 2

This code is will only work properly if you always use that same Context. getPrefs() caches the SharedPreferences instance for ContextA. But if it is called again with ContextB the value returned was the value retrieved when ContextA was used.

---

Using the synchronized modifier on a method synchronizes the method on the class instance (or the class object for static methods). Any code can also synchronize on these instances, potentially causing deadlocks in code you don't control. You should always synchronize on a private instance that only you control.

---

The huge blocks of comments tightly packed in with the code makes everything hard to read. If you are trying to ask for questions about the code, do not put that inside the code. The code you post should be how it would be in the code base.

---

Don't put all of you if blocks on one line. It makes it harder to read and can cause problems when you need to add in a second line.

else if (defaultValue instanceof Float) return (T) (Float) getPrefs(ctx)
            .getFloat(key, (Float) defaultValue);

Here the code is trying to be put on one line, but has to be wrapped. The defeats the whole purpose of trying to one-line the code in the first place. It is best to always put the block on the next line and to always include the curly brackets.