Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Option to get one object instead of list #329

Closed
artem-zinnatullin opened this issue May 15, 2015 · 17 comments
Closed

Option to get one object instead of list #329

artem-zinnatullin opened this issue May 15, 2015 · 17 comments
Assignees
Milestone

Comments

@artem-zinnatullin
Copy link
Member

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
Copy link
Collaborator

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 artem-zinnatullin modified the milestones: v1.1.0, v1.0.0 May 15, 2015
@artem-zinnatullin
Copy link
Member Author

I am moving this to 1.1.0

@artem-zinnatullin
Copy link
Member Author

cc @benjchristensen

@Rainer-Lang
Copy link

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

@geralt-encore
Copy link
Collaborator

@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
Copy link
Member Author

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
#329 (comment)
.

@artem_zin

@geralt-encore
Copy link
Collaborator

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
Copy link
Member Author

@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 😄, 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
Copy link
Collaborator

@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
Copy link
Member Author

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
Copy link
Collaborator

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
Copy link
Member Author

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

@geralt-encore
Copy link
Collaborator

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

@artem-zinnatullin
Copy link
Member Author

@geralt-encore yay! Thanks a lot!

@artem-zinnatullin
Copy link
Member Author

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

@geralt-encore
Copy link
Collaborator

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

@nikitin-da
Copy link
Collaborator

Definitely! Thank you, Ilya!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants