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

Add Single.toBlocking() #3416

Closed

Conversation

artem-zinnatullin
Copy link
Contributor

Closes #3252.

@artem-zinnatullin
Copy link
Contributor Author

Damn, just saw in the issue that @hyleung already implemented it in #3286.

Feel free to close this PR. Though in my impl I've moved common code from BlockingObservable and BlockingSingle into UtilityFunctions and covered it with tests. Also, I've used value() instead of get(), value() looks more consistent with the next(), first(), etc in the BlockingObservable.

I don't mind if @hyleung will reuse my code in his PR :)

* @param subscription
* subscription that needs to be unsubscribed in case of error.
*/
public static void awaitForCompletion(CountDownLatch latch, Subscription subscription) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd put these into a separate class, BlockingUtils or BlockingHelper instead of the UtilityFunctions for better (internal) discoverability.

@akarnokd
Copy link
Member

akarnokd commented Oct 8, 2015

Damn, just saw in the issue that @hyleung already implemented it in #3286.
Yep.

Feel free to close this PR.

This looks cleaner although misses a toFuture method.

* @return a {@code BlockingSingle} version of this Single.
* @see <a href="http://reactivex.io/documentation/operators/to.html">ReactiveX operators documentation: To</a>
*/
public final BlockingSingle<T> toBlocking() {
Copy link
Member

Choose a reason for hiding this comment

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

This should start out as experimental unless the RxJava contributors want to fast-track this.

@akarnokd akarnokd added this to the 1.0.x milestone Oct 8, 2015
@abersnaze
Copy link
Contributor

@hyleung could you review this PR and offer a resolution to the conflict between these two PRs.

}
});

UtilityFunctions.awaitForCompletion(latch, subscription);
Copy link

Choose a reason for hiding this comment

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

On my PR, I had copied awaitForComplete into into BlockingSingle. This is obviously cleaner :p

@artem-zinnatullin
Copy link
Contributor Author

@hyleung you want me to finish this PR or you would like to improve your PR? :) I am okay with any option.

@hyleung
Copy link

hyleung commented Oct 8, 2015

@artem-zinnatullin I'm happy to finish it off on my PR, unless folks feel that it's better to continue with this PR.

@artem-zinnatullin
Copy link
Contributor Author

@hyleung sure! Be ready for my comments on your PR then! 😄

hyleung added a commit to hyleung/RxJava that referenced this pull request Oct 9, 2015
Including:
- renames `BlockingSingle.get()` to `BlockingSingle.value()`
- moves `awaitForComplete()` out into new utility class
  (`rx.internal.util.BlockingUtils`)
- refactors `BlockingSingleTest` to remove the nasty `ExpectedException`
  stuff.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants