Re: [aerogear-dev] [aerogear-android] DataManager API inconsistency + solution

--- Tadeas Kriz tkriz(a)redhat.com <mailto:tkriz@redhat.com> On 08 Jan 2014, at 16:04, Summers Pittman <supittma(a)redhat.com <mailto:supittma@redhat.com>> wrote: > On 01/08/2014 09:57 AM, Tadeas Kriz wrote: >> >> >> On 08 Jan 2014, at 15:53, Summers Pittman <supittma(a)redhat.com >> <mailto:supittma@redhat.com>> wrote: >> >>> On 01/08/2014 09:42 AM, Tadeas Kriz wrote: >>>> >>>> On 08 Jan 2014, at 15:30, Summers Pittman <supittma(a)redhat.com >>>> <mailto:supittma@redhat.com>> wrote: >>>> >>>>> On 01/08/2014 05:51 AM, Tadeas Kriz wrote: >>>>>> Hey everyone, >>>>>> >>>>>> I've been recently going through the DataManager API in >>>>>> aerogear-android. In this email, I'd like to suggest addiction >>>>>> of two method (or possibly three) into the `Store<T>` interface. >>>>>> These would be: >>>>>> >>>>>> ```java >>>>>> /** >>>>>> * If store is open, it can be read or written to. >>>>>> */ >>>>>> boolean isOpen(); >>>>>> >>>>>> /** >>>>>> * Opens store in current thread (blocking). >>>>>> */ >>>>>> Store<T> open(); >>>>>> >>>>>> /** >>>>>> * Opens store in background thread and then callback#onSuccess >>>>>> is called. >>>>>> */ >>>>>> void open(Callback<Store<T>> callback); >>>>>> ``` >>>>> I think those are fine. Feel free to JIRA it up and Passos and I will >>>>> review. >>>>>> >>>>>>> From my point of view, this makes sense to be in the `Store<T>` >>>>>>> so I can switch between stores easily during development with >>>>>>> no need to change other code. Also, if `read` or `write` >>>>>>> operations are done with closed store, there are two possible >>>>>>> workflows. First one is, that I'd fail and throw an exception. >>>>>>> Second (and for me a preferred one) is, that all those methods >>>>>>> would internally check if the store is open and if not, they'd >>>>>>> call the `open` method. This also leads me to another API >>>>>>> change for `Store<T>`. >>>>>> >>>>>> ```java >>>>>> /** >>>>>> * Reads all the data from the underlying storage system >>>>>> asynchronously. >>>>>> */ >>>>>> void readAll(Callback<Collection<T>> callback); >>>>>> >>>>>> /** >>>>>> * Reads a specific object/record from the underlying storage >>>>>> system asynchronously. >>>>>> */ >>>>>> void read(Serializable id, Callback<T> callback); >>>>>> >>>>>> /** >>>>>> * Search for objects/records from the underlying storage system >>>>>> asynchronously. >>>>>> */ >>>>>> void readWithFilter(ReadFilter filter, Callback<List<T>> callback); >>>>>> >>>>>> /** >>>>>> * Saves the given object in the underlying storage system >>>>>> asynchronously. >>>>>> */ >>>>>> void save(T item, Callback<Void> callback); >>>>>> >>>>>> /** >>>>>> * Resets the entire storage system asynchronously. >>>>>> */ >>>>>> void reset(Callback<Void> callback); >>>>>> >>>>>> /** >>>>>> * Removes a specific object/record from the underlying storage >>>>>> system asynchronously. >>>>>> */ >>>>>> void remove(Serializable id, Callback<Void> callback); >>>>>> >>>>>> /** >>>>>> * Checks if the storage system contains no stored elements >>>>>> asynchronously. >>>>>> */ >>>>>> void isEmpty(Callback<Boolean> callback); >>>>>> ``` >>>>>> >>>>>> That's right, async methods for easy access to the storage from >>>>>> background thread, without the pain of writing it myself (for >>>>>> example, it makes no sense if I want to just call >>>>>> `store.save(..)` and I'd have to write all the `AsyncTask` >>>>>> boilerplate). >>>>>> >>>>>> So, what do you think? >>>>> >>>>> I would rather throw an exception than open a database when you call >>>>> read and friends. That way a developer doesn't accidentally open a >>>>> database he meant to be closed. I don't have that strong of a >>>>> feeling on >>>>> that point one way or another however. >>>> >>>> That's right, it's probably less error prone in scenarios when you >>>> want the store closed. >>>> >>>>> >>>>> My stronger feeling is on adding callbacks to the stores methods. I >>>>> prefer for the Store to be synchronous and Pipes to be >>>>> asynchronous. We >>>>> could add a StorePipe to our PypeTipes which may solve some of >>>>> the headache. >>>>> >>>> >>>> Would "void open(Callback<Store<T>> callback);" make sense then? I >>>> mean, that would add another inconsistency in the API, as one >>>> method would be async and the rest would be only synchronous, >>>> wouldn't it? >>> True. The reason for the exception here was that opening a SQL >>> store or an encrypted store COULD take significant amount of time. >>> For in Memory data stores this is instant of course. >> I understand, but that's just an assumption. Let's say that there's >> a storage, that'd take an instant to load data from, but a lot of >> time to save data (like XML file based store). What's the main >> reason to not have async "read"/"write" methods? > > The best reason I can give is if your Store implementation will be > that slow either a) Write a Pipe instead or b) Manage the latency > yourself. > > In Android land you want code on your main thread to be fast or non > blocking. If we are working off the main thread then having things > be slow and blocking is less of an issue. Synchronous code is easier > to debug (in Java) and easier to write (in Java) and faster (because > we don't have to route through callback classes). > > Having either or, in my opinion, clouds what your usage target should be. > > Also this is the most though Stores have been given in a long time. > Anything we come up with we will probably need to get the other > platforms alerted to as well. > Well, sometimes you want to reload a ListView data and it'd make it so easy to just call "store.readAll(callback)" where in callback#onSuccess you'd remove progress indicator and notify the ListView about new data. BTW, I've been working on new DataManager API, which would be a lot more interesting for users (right now the need to cast it and also that it's not easily extendable is a pain), so when I'm done with that, I'll start another thread here on ML ;)

> >>>> >>>>> Passos, wdyt? >>>>> >>>>> >>>>>> >>>>>> PS: You can find the whole text with highlighted syntax here: >>>>>> >>>>>> --- >>>>>> Tadeas Kriz >>>>>> tkriz(a)redhat.com <mailto:tkriz@redhat.com> >>>>>> >>>>>> >>>>>> _______________________________________________ >>>>>> aerogear-dev mailing list >>>>>> aerogear-dev(a)lists.jboss.org <mailto:aerogear-dev@lists.jboss.org> >>>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev >>>>> >>>>> _______________________________________________ >>>>> aerogear-dev mailing list >>>>> aerogear-dev(a)lists.jboss.org <mailto:aerogear-dev@lists.jboss.org> >>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev >>>> >>>> >>>> >>>> _______________________________________________ >>>> aerogear-dev mailing list >>>> aerogear-dev(a)lists.jboss.org >>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev >>> >>> _______________________________________________ >>> aerogear-dev mailing list >>> aerogear-dev(a)lists.jboss.org <mailto:aerogear-dev@lists.jboss.org> >>> https://lists.jboss.org/mailman/listinfo/aerogear-dev >> >> >> >> _______________________________________________ >> aerogear-dev mailing list >> aerogear-dev(a)lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/aerogear-dev > > _______________________________________________ > aerogear-dev mailing list > aerogear-dev(a)lists.jboss.org <mailto:aerogear-dev@lists.jboss.org> > https://lists.jboss.org/mailman/listinfo/aerogear-dev _______________________________________________ aerogear-dev mailing list aerogear-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/aerogear-dev