Skip to content

Implemented Completable#andThen(Observable) #3570

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

Merged
merged 1 commit into from
Jan 4, 2016

Conversation

stealthcode
Copy link

I expect some discussion around the method name.

@stealthcode
Copy link
Author

Adding unit tests and pushing shortly.

*/
public final <T> Observable<T> andThen(Observable<T> next) {
requireNonNull(next);
return next.delaySubscription(toObservable());
Copy link
Member

Choose a reason for hiding this comment

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

Completable already has an endWith overload with the same purpose.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, I missed that.

Copy link
Member

Choose a reason for hiding this comment

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

Is there a verdict on Observable.andThen? Renaming endWith to andThen may serve some consistency purpose among base classes.

Copy link
Author

Choose a reason for hiding this comment

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

One initial motivation for andThen was based on the Option::and_then method in Rust stdlib. It chains through the computation and transforms it's type/contents to the next Option.

Copy link
Contributor

Choose a reason for hiding this comment

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

My first thought was java.util.function.Function a; a.andThen(b). Also endWith doesn't end the composition this would look strange a.endWith(b).endWith(c) vs a.andThen(b).andThen(c).

Copy link
Member

Choose a reason for hiding this comment

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

I think andThen is more idiomatic than endWith, especially for chaining purposes.

Copy link
Author

Choose a reason for hiding this comment

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

I guess that if we are going to rename or change any of the public API on Completable now is the time before our next release. Should we aim to review the operators before next release?

Copy link
Member

Choose a reason for hiding this comment

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

Yes.

TestSubscriber<String> ts = new TestSubscriber<>(0);
AtomicBoolean hasRun = new AtomicBoolean(false);
Exception e = new Exception();
Completable.create(cs -> cs.onError(e))
Copy link
Member

Choose a reason for hiding this comment

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

Lambdas are Java 8. Besides, there is already a Completable.error() and you should call cs.onSubscribe(Subscriptions.unsubscribed()) first.

@akarnokd
Copy link
Member

akarnokd commented Jan 4, 2016

👍

akarnokd added a commit that referenced this pull request Jan 4, 2016
Implemented Completable#andThen(Observable)
@akarnokd akarnokd merged commit 0ea7bcf into ReactiveX:1.x Jan 4, 2016
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