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

Tadeas Kriz tkriz at redhat.com
Wed Jan 8 10:09:58 EST 2014 

—
Tadeas Kriz
tkriz at redhat.com

On 08 Jan 2014, at 16:04, Summers Pittman <supittma at redhat.com> wrote:

> On 01/08/2014 09:57 AM, Tadeas Kriz wrote:
>> 
>> 
>> On 08 Jan 2014, at 15:53, 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.
>> 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 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
> 
> _______________________________________________
> 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/a3521107/attachment.html

More information about the aerogear-dev mailing list