Improving usage of shared preferences

[Bluesir9]

I think the way shared preferences have been used can be improved. Ideally we could have a singleton class to manage our usage of shared preferences with all the keys used across the project put together in a single enum. That way it would become simple to track what all keys have been used all across.

[mashawan]

Hi, can I take this? I was working with shared preferences in my last issue, I should be able to improve things.

[Bluesir9]

I was kinda waiting on someone with more experience with the project to provide their feedback on this before picking this up :)

[neslihanturan]

Hi, this improvement is very meaningful. Actually, we have talked about the same thing on pre-hackathon and I have implemented it. However, since I couldn't resolve conflicts for long time, I closed my own pr. Sometimes rewriting it is better than solving several conflicts:) so feel free to work on it @mashawan

[neslihanturan]

Here #609

[neslihanturan]

Hi @mashawan are you still working on this issue? If not I can re-promote this issue as almost beginner friendly:)

[tshradheya]

I will take a stab on this

[neslihanturan]

Thanks @tshradheya , please take a look at #609 . It displays what was expected for this PR. Besides, this task requires testing carefully. Please note all shared preferences when refactoring and test all of them at the end. It would be great if you note them on your PR, so that we can test easier.

[tshradheya]

Hi, I started working on this issue (sorry for the delay).

I realized that the situation is no longer as in #609. Now different prefs are used and are Injected. For e.g.

    @Inject @Named("prefs") SharedPreferences prefs;
    @Inject @Named("default_preferences") SharedPreferences defaultPrefs;

is not used instead of just one SharedPreferences like before

[psh]

@tshradheya - the Injection just highlights a problem that was hidden before. There always were three different sets of shared preferences in play and the move to inject preferences with Dagger makes it much clearer as to which ones are used where. I agree that the code has changed since #609 but I believe that the changes bring clarity that wasn't there before.

In the old code, there were 3 ways to retrieve preferences

1. context.getSharedPreferences("prefs", MODE_PRIVATE)
2. PreferenceManager.getDefaultSharedPreferences(context)
3. context.getSharedPreferences("fr.free.nrw.commons", MODE_PRIVATE)

These correspond with named injections today

1. @Inject @Named("prefs")
2. @Inject @Named("default_preferences")
3. @Inject @Named("application_preferences")

[maskaravivek]

As suggested by @psh we have already moved to @Named injections in most of the places. To make the usage cleaner, I propose the usage of a wrapper class for interacting with any Shared Preference. The wrapper should implement the following interface:

public interface KeyValueStore {
    String getString(String key);
    boolean getBoolean(String key);
    long getLong(String key);
    int getInt(String key);
    void putString(String key, String value);
    void putBoolean(String key, boolean value);
    void putLong(String key, long value);
    void putInt(String key, int value);
    boolean contains(String key);
    void remove(String key);
    void clearAll();
    void clearAllWithVersion();
}

Also, a JsonKvStore can extend a BasicKvStore and let us store whole objects in shared prefs.

I would like to work on this refactoring.