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

[Summers Pittman]

On 01/08/2014 09:59 AM, Lucas Holmquist wrote:
>
> On Jan 8, 2014, at 9:53 AM, 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.
>
> In JS,  once we added IndexedDB and WebSql which are async,  we 
> deprecated the sync api and made everything "async"
I didn't realize you guys were deprecating the sync methods.  I thought 
you were just adding the aysnc methods because that* is what JS 
developers expect.



*That = callback hell :-p
>
>>>
>>>> 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/e1eef34e/attachment-0001.html