pushtorefresh / storio

Reactive API for SQLiteDatabase and ContentResolver.
Apache License 2.0
2.55k stars 182 forks source link

Option to get one object instead of list #329

Closed artem-zinnatullin closed 8 years ago

artem-zinnatullin commented 9 years ago

Common task — get exactly one object from the db.

// Can be null.
final User user = storIOSQLite 
  .get()
  .object(User.class)
  .withQuery(new Query.Builder()
    .table("users")
    .where("user_id = ?")
    .whereArgs(userId)
    .build())
  .prepare()
  .executeAsBlocking()

I think, it should return null if cursor.getCount() == 0 for executeAsBlocking(), for RxJava it should just complete normally without emitting null, so it'll be possible to apply defaultOrEmpty Operator.

storIOSQLite 
  .get()
  .object(User.class)
  .withQuery(new Query.Builder()
    .table("users")
    .where("user_id = ?")
    .whereArgs(userId)
    .build())
  .prepare()
  .createObservable()
  .defaultIfEmpty(loadUserFromNetwork(userId)) // elegant?

@nikitin-da what do you think?

nikitin-da commented 9 years ago

Yes, I agree that 'null' for blocking variant will be obvious. For rx, we can simply replace 'loadUserFromNetwork' with swithing ui state to empty in main thread, don,t we?

artem-zinnatullin commented 9 years ago

I am moving this to 1.1.0

artem-zinnatullin commented 9 years ago

cc @benjchristensen

Rainer-Lang commented 9 years ago

Would be nice to have this. I really need it.

geralt-encore commented 9 years ago

@artem-zinnatullin Hey! I have done this feature for the executeAsBlocking case, but not for createObservable yet, because of a lack of experience with RxJava on that level. So should I make a PR? And if yes what base branch should I choose? It is my first serious contribution to open source and I am very excited about it =), so sorry for my newbie questions.

artem-zinnatullin commented 9 years ago

Hi, Ilya!

Feel free to open a PR! First contribution is always exciting, we would be happy to help you with it :)

Sorry for the delay, we are quite busy this week.

On Tue, Nov 17, 2015, 20:43 Ilya notifications@github.com wrote:

@artem-zinnatullin https://github.com/artem-zinnatullin Hey! I have done this feature for the executeAsBlocking case, but not for createObservable yet, because of a lack of experience with RxJava on that level. So should I make a PR? And if yes what base branch should I choose? It is my first serious contribution to open source and I am very excited about it =), so sorry for my newbie questions.

— Reply to this email directly or view it on GitHub https://github.com/pushtorefresh/storio/issues/329#issuecomment-157449034 .

@artem_zin

geralt-encore commented 9 years ago

Hi, Artem! I have started to implement Rx part and faced with some difficulties because a lack of experience with it, so I need your advice. I tried to deal with it in that way:

storIOSQLite.observeChangesInTables(tables)
                .map(MapSomethingToExecuteAsBlocking.newInstance(this))
                .startWith(Observable.create(OnSubscribeExecuteAsBlocking.newInstance(this)))
                .filter(new Func1<T, Boolean>() {
                    @Override
                    public Boolean call(T t) {
                        return t != null;
                    }
                })
                .first()
                .subscribeOn(Schedulers.io());

The problem is that in the case when there are no objects satisfying query - observable never completes and it is impossible to use defaultIfEmpty for example. I am probably missing something really simple, but I haven't discovered it yet. I'll very appreciate your advice.

artem-zinnatullin commented 9 years ago

@geralt-encore cool!

So, you should not filter() the emission in this case because you're just skipping nulls now and it's incorrect behavior. Also, first() will emit only one result and then Observable will be completed.

Looks like you understand Russian :smile:, so you can try to watch my talk about Rx and it'll hopefully help you understand Rx better https://events.yandex.ru/lib/talks/3106/

I'd rewrite this to something like:

observeChangesInTables(tables)
  .map(MapSomethingToExecuteAsBlocking.newInstance(this)) // On each change get new result
  .startWith(Observable.create(OnSubscribeExecuteAsBlocking.newInstance(this))) // Start with initial result
  .subscribeOn(Schedulers.io());

Should work fine!

geralt-encore commented 9 years ago

@artem-zinnatullin I am not sure that I explained my question well. I was talking about the implementation of executeAsBlocking in PreparedGetObject. I am using first because I need take only one object. And I need to use filter because executeAsBlocking can return null in the case when there is no object that satisfies query was found. Is it approach completely wrong?

artem-zinnatullin commented 9 years ago

first (same as take(1)) will take one result of emission but then we won't receive updates on table changes.

null is valid result for executeAsBlocking(), so it should be the same for rx approach :)

geralt-encore commented 9 years ago

Oh, now I understand the problem with first. So are you sure about emitting null and leaving to end users to handle that case?

artem-zinnatullin commented 9 years ago

But we already do this for executeAsBlocking, Rx support should provide same results.

geralt-encore commented 8 years ago

@artem-zinnatullin Looks like this issue can be closed =)

artem-zinnatullin commented 8 years ago

@geralt-encore yay! Thanks a lot!

artem-zinnatullin commented 8 years ago

It was pleasure to merge your PRs! Hope @nikitin-da thinks so too :smile:

geralt-encore commented 8 years ago

Thanks! I've learned a lot from you guys and looking forward to continuing to contribute into StorIO

nikitin-da commented 8 years ago

Definitely! Thank you, Ilya!