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

[Tadeas Kriz]

—
Tadeas Kriz
tkriz at redhat.com

On 08 Jan 2014, at 15:59, Lucas Holmquist <lholmqui at redhat.com> wrote:

> 
> On Jan 8, 2014, at 9:53 AM, Summers Pittman <supittma at redhat.com> wrote:
> 
>> On 01/08/2014 09:42 AM, Tadeas Kriz wrote:
>>> 
>>> On 08 Jan 2014, at 15:30, Summers Pittman <supittma at 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.
> 
> In JS,  once we added IndexedDB and WebSql which are async,  we deprecated the sync api and made everything "async"
Well, from my experience with Android development, you don’t always want everything async. You should be able to easily select. Sometimes you're already in background thread, so you want the data to be loaded/saved synchronously.
>>> 
>>>> Passos, wdyt?
>>>> 
>>>> 
>>>>> 
>>>>> PS: You can find the whole text with highlighted syntax here:
>>>>> 
>>>>> —
>>>>> Tadeas Kriz
>>>>> tkriz at redhat.com
>>>>> 
>>>>> 
>>>>> _______________________________________________
>>>>> aerogear-dev mailing list
>>>>> aerogear-dev at lists.jboss.org
>>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>>> 
>>>> _______________________________________________
>>>> aerogear-dev mailing list
>>>> aerogear-dev at lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>> 
>>> 
>>> 
>>> _______________________________________________
>>> aerogear-dev mailing list
>>> aerogear-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>> 
>> _______________________________________________
>> aerogear-dev mailing list
>> aerogear-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
> 
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/aerogear-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/aerogear-dev/attachments/20140108/e0a7ba79/attachment.html