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

1.x: Single.flatMapCompletable #4226

Merged
merged 7 commits into from
Jul 27, 2016
Merged

1.x: Single.flatMapCompletable #4226

merged 7 commits into from
Jul 27, 2016

Conversation

vanniktech
Copy link
Collaborator

Addresses #4216

Happy to receive feedback on the implementation. Also should I take the documentation from flatMapObservable and adjust it?

@codecov-io
Copy link

codecov-io commented Jul 22, 2016

Current coverage is 84.19% (diff: 100%)

Merging #4226 into 1.x will increase coverage by 0.06%

@@                1.x      #4226   diff @@
==========================================
  Files           265        266     +1   
  Lines         17305      17335    +30   
  Methods           0          0          
  Messages          0          0          
  Branches       2624       2625     +1   
==========================================
+ Hits          14559      14596    +37   
+ Misses         1893       1887     -6   
+ Partials        853        852     -1   

Powered by Codecov. Last update 479df31...3aad33f

@akarnokd
Copy link
Member

The unsubscription is not properly linked, the possible exception thrown by the mapper is not handled.

See this.

Also should I take the documentation from flatMapObservable and adjust it?

Yes.

@@ -1396,6 +1396,25 @@ public R call(Object... args) {
return Observable.merge(asObservable(map(func)));
}

public final Completable flatMapCompletable(final Func1<? super T, ? extends Completable> func) {
Copy link
Contributor

Choose a reason for hiding this comment

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

javadoc

@vanniktech
Copy link
Collaborator Author

Updated the documentation. I hope the wording is okay now. Also I'm pointing to https://raw.githubusercontent.com/wiki/ReactiveX/RxJava/images/rx-operators/Single.flatMapCompletable.png which does not exist yet but @DavidMGross already tracked it at ReactiveX/reactivex.github.io#289, I hope that's okay.

thanks for the CompletableFlatMapSingleToCompletable I knew something was missing. I also created two more tests that cover some of the previously missing behaviour.

* @see <a href="http://reactivex.io/documentation/operators/flatmap.html">ReactiveX operators documentation: FlatMap</a>
*/
public final Completable flatMapCompletable(final Func1<? super T, ? extends Completable> func) {
return Completable.create(new CompletableFlatMapSingleToCompletable(this, func));
Copy link
Member

Choose a reason for hiding this comment

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

You forgot to commit the class itself. Plus, use new CompletableFlatMapSingleToCompletable<T>(this, func) as it is a generic class.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup I just comitted it. Got confused since I had to stage the files individually because of the star imports ...


final Single<T> source;

final Func1<? super T, Completable> mapper;
Copy link
Member

Choose a reason for hiding this comment

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

You have to change this to ? extends Completable (and update the other places) so it compiles with the operator signature in Single

* @return the Completable returned from {@code func} when applied to the item emitted by the source Single
* @see <a href="http://reactivex.io/documentation/operators/flatmap.html">ReactiveX operators documentation: FlatMap</a>
*/
public final Completable flatMapCompletable(final Func1<? super T, ? extends Completable> func) {
Copy link
Member

Choose a reason for hiding this comment

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

Please tag this as @Experimental.

@akarnokd
Copy link
Member

👍

@vanniktech
Copy link
Collaborator Author

Would it be possible to get this one into 1.1.8?

@akarnokd
Copy link
Member

If @zsxwing or anyone from Netflix approves it in time.

@@ -1397,6 +1397,29 @@ public R call(Object... args) {
}

/**
* Returns a Completable that completes based on applying a specified function to the item emitted by the
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: {@link Completable}

@artem-zinnatullin
Copy link
Contributor

👍

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