Skip to content
This repository has been archived by the owner on Aug 17, 2020. It is now read-only.

Optionally map a SqlBrite.Query to an Observable #45

Merged
merged 2 commits into from Aug 30, 2015

Conversation

AlecKazakova
Copy link
Member

No description provided.

@JakeWharton
Copy link
Member

I was thinking about this on the plane today, what if we return a subclass of Observable (say, QueryObservable) which exposes a <T> Observable<T> mapToModel(Func1<Cursor, T> funky) method. Your code then becomes:

db.createQuery("SELECT * FROM users", "users")
  .mapToModel(User.MAP)
  .toList()
  .subscribe(adapter);

@AlecKazakova
Copy link
Member Author

Not sure that will work - toList() requires a onComplete that it's never going to get. All the logic needs to live inside a flatMap (I think). I like the idea of having something exposed at the observable level though

@JakeWharton
Copy link
Member

Ah, right. Well it can just be

db.createQuery("SELECT * FROM users", "users") // Observable<Cursor>
  .mapToModel(User.MAP) // Observable<List<User>>
  .subscribe(adapter);

then.

@JakeWharton
Copy link
Member

public <T> Observable<List<T>> mapToModel(final Func1<Cursor, T> func) {
  return lift(new Observable.Operator<List<T>, Query>() {
    @Override public Subscriber<? super Query> call(final Subscriber<? super List<T>> subscriber) {
      return new Subscriber<Query>(subscriber) {
        @Override public void onNext(Query query) {
          Cursor cursor = query.run();
          List<T> items;
          try {
            items = new ArrayList<>(cursor.getCount());
            while (cursor.moveToNext()) {
              items.add(func.call(cursor));
            }
          } finally {
            cursor.close();
          }
          subscriber.onNext(items);
        }

        @Override public void onCompleted() {
          subscriber.onCompleted();
        }

        @Override public void onError(Throwable e) {
          subscriber.onError(e);
        }
      };
    }
  });
}

@artem-zinnatullin
Copy link
Contributor

while (cursor.moveToNext()) -> while (cursor.moveToNext() && !subscriber.isUnsubscribed())

@AlecKazakova
Copy link
Member Author

By going straight into the list it's removing a lot of potential functionality that this whole PR was intended to help add. Some examples:

db.createQuery(User.TABLE_NAME, "SELECT * FROM user")
  .flatMap(query -> query.map(User.MAP)
    .filter(user -> user instanceof CoolUser)
    .toList())
  .subscribe(adapter);
db.createQuery(User.TABLE_NAME, "SELECT * FROM user")
  .flatMap(query -> query.map(User.MAP)
    .toSortedList((lhs, rhs) -> (lhs.firstName.compateTo(rhs.firstName))
  .subscribe(adapter);

Or say you want to collect the users into a set, turn them into a map/multimap, etc. They're all possible with mapToModel by doing something like

db.createQuery(User.TABLE_NAME, "SELECT * FROM user")
  .mapToModel(User.MAP)
  .flatMap(list -> Observable.from(list)
    ...
    .toList())
  .subscribe(adapter);

but I think we should be helping out the other case. Plus then we're not composing a list, splitting it out, and composing it back together again.

@JakeWharton
Copy link
Member

Ah, right right. The conversation is coming back to me now.

@JakeWharton
Copy link
Member

We should still do a mapToList similar to what I posted in a follow-up because the examples in the sample app are going to be the 99% case and right now they're boilerplate.

while (cursor.moveToNext()) {
subscriber.onNext(map.call(cursor));
}
subscriber.onCompleted();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call this outside the try so if close() throws it'll go to onError

@AlecKazakova
Copy link
Member Author

Yup definitely. Plus since they're taking the same Func1<Cursor, T> then people can use the two API's interchangeably which is nice.

*
* @param map Takes in a cursor for a single row and maps it to your desired result type.
*/
public <T> Observable<T> map(final Func1<Cursor, T> map) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

final

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how I feel about overloading map... Maybe "asRows" or "toRows" or something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, map makes it sound like you're mapping the whole query to something. asRows sounds good.

@JakeWharton
Copy link
Member

LGTM

AlecKazakova pushed a commit that referenced this pull request Aug 30, 2015
Optionally map a SqlBrite.Query to an Observable
@AlecKazakova AlecKazakova merged commit 20eede8 into square:master Aug 30, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants