clojure-android / neko

The Clojure/Android Toolkit
Other
297 stars 36 forks source link

[n.data] Define defpreference, initialize-preferences and auxiliaries #55

Closed Malabarba closed 9 years ago

Malabarba commented 9 years ago

This PR defines a defpreference macro. Using this macro to define a variable will define an atom instead, and its value will be recorded throughout sessions, assuming initialize-preferences is called at some point.

This is an initial proposal, let me know what you think of the implementation. In particular, there's no real need for the sp variable. I just included it in case the user wants to edit some values manually, but it could be removed.

alexander-yakushev commented 9 years ago

A couple of immediate comments without reading too much into it:

Overall, I like the idea, but the implementation is somewhat questionable. It is confusing that you have to defpreference something and then call initialize on it later. It is two API functions that cannot go without each other. I would rather prefer an approach where you define a simple atom in top level, and then (bind atom "prefs") in onCreate.

Malabarba commented 9 years ago
  • this requires a usage example, you can paste it here in the comments. The example will go to the wiki.

Sure, I'll write one as soon as we settle on the implementation.

  • versioning seems an overkill to me. Unlike SQL it is quite easy to clear a sharedpref file. In this case it is even easier, just reset the atom, right?

Well, lets say you change a preference from being a number to being a string, you only want to reset the user's preference if it is still using the old implementation.

In this case, the version parameter is a shorter way of doing (when (number? @the-pref) (reset! the-pref “default-value”)). But this snippet would have to be called after initialization, so it would end up going in the app's onCreate instead of in the variable's definition (where it should be).

  • I don't like that it uses assoc-arbitrary! for all values. I've written it some time ago, I'm not very fond of it now, and even then I supposed it to be a hack for an occasional small vector. It uses reader, reader is bad.

Ok. Would it be better to only use assoc-arbitrary! if a keyword option is provided? FWIW, it takes care to disregard unreadable values instead of letting the exception raise.

Overall, I like the idea, but the implementation is somewhat questionable. It is confusing that you have to defpreference something and then call initialize on it later.

Just to be clear, you don't call initialize on the preference, you just call a single (initialize-preferences this “my-prefs”) which initializes all of them (with this being the activity).

It is two API functions that cannot go without each other. I would rather prefer an approach where you define a simple atom in top level, and then (bind atom "prefs") in onCreate.

This would be more verbose, but it also has the advantage of consolidating all preferences specifications in one place. Shall I change to this, then?

alexander-yakushev commented 9 years ago

Sure, I'll write one as soon as we settle on the implementation.

OK, I think it will be better if you provide the example for the current implementation now, because apparently I haven't completely understand how it works and what it should do.

Malabarba commented 9 years ago

Here we go.

;; On the surface, this is equivalent to (def username (atom "Boogie")).
(defpreference username "Boogie")
(defpreference user-age 50)
;; The defpreferences can go in any file, as long as they are loaded
;; by the time `initiaze-preferences` is called (though it is possible
;; to remove that limitation).

;; Then, in your main activity
(defactivity com....MainActivity
  :on-create
  (fn [this bundle]
      ;; This is the point where the SharedPreferences object is
      ;; created, where previously saved values are restored, and
      ;; where we start tracking any changes.
      (neko.data/initialize-preferences this "some-tag")))
Malabarba commented 9 years ago

The above example will:

  1. Define username to be (atom “boogie”). And similarly for user-age
  2. When the initialize-preferences is called:
    1. Get a SharedPreferences object for the app.
    2. Check if username had a saved value and, if it did, reset! the atom to the saved value. Similarly for user-age.
    3. Set a watch on username and user-age.
  3. In the future, whenever the atoms are edited, save the new value in the SharedPreferences object.
alexander-yakushev commented 9 years ago

I see now why I was confused. Why do you want to split the preferences into separate vars with individual atoms? That is not very scalable if I have, say, 10 keyvalues in a SharedPreferences file.

I would rather have a single atom that represents the entire SP file. There are multiple advantages:

So my implementation would look something like:

(def prefs (atom {}))

(defactivity com....MainActivity
  :key :main
  (onCreate [this bundle]
    (.superOnCreate this bundle)
    (neko.data/bind-preferences prefs "my-prefs")))

BTW, notice the new syntax for defactivity. It sits in 4.0 branch right now, and the old one will be deprecated upon release (but that won't happen very soon I think:)).

Malabarba commented 9 years ago

I see now why I was confused. Why do you want to split the preferences into separate vars with individual atoms?

The idea is that you already have atoms as part of your clojure code, so this just allows you save their values throughout sessions without having to change the way you use them. You just use and/or edit the atom like you normally would in regular clojure code, with the only difference being that its initial value will be automatically taken from a previously saved value.

I guess, in a way, it's about adapting SharedPreferences into clojure syntax, rather writing a clojure syntax for SharedPreferences.

That is not very scalable if I have, say, 10 keyvalues in a SharedPreferences file.

Why not? For each mutable variable you want to keep saved, you just change the def ... (atom ...) into a defpreference.

I would rather have a single atom that represents the entire SP file. There are multiple advantages:

  • You just have to define the atom once per SP file
  • Atom's transactional model and SP's update-commit models are similar, almost 1-to-1 mapping.
  • Ability to see whole SP file at once by just inspecting the atom.

I think I see our disagreement, we just have different applications in mind.

Correct me if I'm wrong, I think what you have in mind is a convenient clojure interface for editing/reading an SP file, whereas I'm just thiking of a concise way to remember the value of certain variables between sessions. Of course the former is a relevant use-case, it's just not what I focused on (because I just don't need it ATM). For the purpose of the latter, you have no reason to worry about the SharedPreferences file.

  • You can use multiple SP files. In your example I don't see how defpreference can say which SP file it belongs to.

This can be made to be a keyword argument. Like I said, managing the file itself just wasn't the focus.

  • There is no mutable state in neko.data, it doesn't have to keep track of anything after the binding is initialized
  • Less unknown API.
  • No confusion about whether top-level defpreference does anything Android-related when executed.

The application I suggested can (and shall) be changed to fix these points too. For instance, it could use the following syntax.

 (def username (atom “Boogie”))

 (defactivity com....MainActivity
   :key :main
   (onCreate [this bundle]
     (.superOnCreate this bundle)
     (neko.data/restore-and-track username :username "my-prefs")))

Note that this doesn't prevent the user from having other SP files that are managed in different ways or for different purposes. It just allows you to have an SP file which restores and tracks several atoms for you such that you never have to worry about them.

Malabarba commented 9 years ago

Of course, if you think this usage of "just keeping some variables tracked" is too specific to go into neko, then that's perfectly fine. :-)

alexander-yakushev commented 9 years ago

All right. I get what are you trying to do here. The problem of unreliable state in mobile applications is quite novel if you think of it. Desktop apps aren't written with consideration that they can be paused or killed at any time, then launched again and they should work like nothing have happened. Coming to terms with this either requires a paradigm shift in developer's mind, or a robust tool that enables the old ways in a new environment.

I've thought about it for a long time. Android provides a lot of ways to preserve state between pauses/relaunches, namely: Fragments remembering the state of their UI, Intent data, Activity bundles, SharedPreferences, SQLite. It is often unclear which one to use in the particular situation. I actually wonder if a new type of language has to be created for this "application ephemerality", like Erlang was to concurrent programming.

Anyway, even in desktop/server Clojure there is a push towards more centralized state (cue component). I don't think keeping the state spread out across different vars and atoms is a healthy approach, neither on Android, nor in JVM.

So you don't want to just wrap SP, you want to create a state abstraction that preserves Clojure data between relaunches, with SP being an implementation detail. But you can't really do that, you can't just wave away SP and semantically detach from it. The abstraction is too leaky. What happens if the user puts a 10000-element map in your var? Trouble. A closure? Big trouble. So you'd have to explicitly tell the users what kind of values are acceptable, at which point the abstraction is effectively gone and they are using SharedPreference piecewise.

In conclusion, you gave me a great idea of wrapping SP into an atom+watch which I gonna implement in Neko soon (with attribution no less:)). But I don't feel that your impl is a sufficient solution for the bigger problem. However, I'm highly interested in solving that bigger problem, so if you have new developments on it please keep me updated!

Malabarba commented 9 years ago

Ok. I see your point, and I do agree with it.

My only caveat is that telling the developer which values he can use on a preference doesn't invalidate the benefits offered by the system. It does break the abstraction somewhat, but the point of this system is to be an interface, not an abstraction (sure, I was abstracting away the SP file itself, but not the fact that we're using SP). In other words, you get to use some standard clojure paradigms as a simplified interface to Android SPs. You don't get to pretend this is not Android.

In any case, I agree with the rest. The system you've been hinting on does seem much more robust and complete, so it would certainly be better. :)

alexander-yakushev commented 9 years ago

Yes, I have probably overblown the extent you "hide" sharedpreferences.

Anyway, thanks for the atom idea! I think it is really neat, and gonna be much easier to use than to pull/push fields to SP directly.