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

[Summers Pittman]

On 01/08/2014 09:57 AM, Tadeas Kriz wrote:
>
>
> On 08 Jan 2014, at 15:53, Summers Pittman <supittma at redhat.com 
> <mailto: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 
>>> <mailto: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.


>>>
>>>> Passos, wdyt?
>>>>
>>>>
>>>>>
>>>>> PS: You can find the whole text with highlighted syntax here:
>>>>>
>>>>> ---
>>>>> Tadeas Kriz
>>>>> tkriz at redhat.com <mailto:tkriz at redhat.com>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> aerogear-dev mailing list
>>>>> aerogear-dev at lists.jboss.org <mailto:aerogear-dev at lists.jboss.org>
>>>>> https://lists.jboss.org/mailman/listinfo/aerogear-dev
>>>>
>>>> _______________________________________________
>>>> aerogear-dev mailing list
>>>> aerogear-dev at lists.jboss.org <mailto: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 <mailto: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/6529fdd0/attachment-0001.html