-
Notifications
You must be signed in to change notification settings - Fork 3k
Adding support to create self closing Observables from Cursors #76
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
Conversation
@mttkay @JakeWharton this pull request is for #71 |
/** | ||
* This class adapts a Cursor into an Iterable. | ||
*/ | ||
public class CursorIterableAdapter implements Iterable<Cursor> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the value in this abstraction? Why not just write a normal subscribe func?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I was trying to leverage the Observable.from(Iterable) but looking at it now if I follow your suggestion there will be probably less code overall anyway.
@dpsm have you checked out Venmo's |
@ronshapiro yeah I agree we should make people aware of that type of stuff that added to RX makes things easy 👍 I have updated the pull request with the simplest and safest implementation I could think of. |
Needs a rebase and squashed. I can take care of it in about 30m if you haven't done it by then. |
@JakeWharton I am on it :) |
This commit removes CursorIterableAdapter.java and implements the Cursor navigation and safe closing in one place. The option to use doOnTerminate(..) will not work as well to close the Cursor since downstream operators while emiting errors would not pass through the operator closing the Cursor. Signed-off-by: David Marques <dpsmarques@gmail.com>
Adding support to create self closing Observables from Cursors
I wonder what the tests for this would have looked like with |
I can put something together based on matrix cursor with a few test rows
|
It shouldn't matter. There's already a ton of tests which are written in the same way. Besides, we end up relying on Robolectric's implementation rather than a real implementation (although in this case I'm pretty sure it's the real code). |
Any idea when this will be available from the maven central? Maven Central shows that the version is 0.23.0 which is updated 13-Nov-'14. So rx.android.content.ContentObservable is not available until now (first commit was on 22-Nov). |
An optional overriden method allows a mapper function to create
instances of a type from the Cursor as the Observable iterates
over it.
Signed-off-by: David Marques dpsmarques@gmail.com