-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
2.x Add concatMapCompletable() to Observable #5649
Conversation
Codecov Report
@@ Coverage Diff @@
## 2.x #5649 +/- ##
============================================
- Coverage 96.19% 96.15% -0.05%
- Complexity 5843 5849 +6
============================================
Files 634 635 +1
Lines 41539 41652 +113
Branches 5752 5768 +16
============================================
+ Hits 39960 40050 +90
- Misses 626 640 +14
- Partials 953 962 +9
Continue to review full report at Codecov.
|
* @param mapper | ||
* a function that, when applied to an item emitted by the source ObservableSource, returns a CompletableSource | ||
* @return a Completable that signals {@code onComplete} when the upstream and all CompletableSources complete | ||
*/ |
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.
Please add @since 2.1.6 - experimental
*/ | ||
@CheckReturnValue | ||
@SchedulerSupport(SchedulerSupport.NONE) | ||
public final Completable concatMapCompletable(Function<? super T, ? extends CompletableSource> mapper) { |
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.
Please add @Experimental
.
* @param prefetch | ||
* the number of elements to prefetch from the current Observable | ||
* @return a Completable that signals {@code onComplete} when the upstream and all CompletableSources complete | ||
*/ |
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.
Please add @since 2.1.6
.
*/ | ||
@CheckReturnValue | ||
@SchedulerSupport(SchedulerSupport.NONE) | ||
public final Completable concatMapCompletable(Function<? super T, ? extends CompletableSource> mapper, int prefetch) { |
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.
Please add @Experimental
.
* | ||
* @param mapper | ||
* a function that, when applied to an item emitted by the source ObservableSource, returns a CompletableSource | ||
* @param prefetch |
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.
A capacityHint
would capture the underlying behavior better: "the number of upstream items expected to be buffered until the current CompletableSource, mapped from the current item, completes."
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.
Codewise it looks okay. /cc @artem-zinnatullin, @vanniktech & @davidmoten who was also interested in this.
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.
Should we also include a diagram?
@CheckReturnValue | ||
@SchedulerSupport(SchedulerSupport.NONE) | ||
public final Completable concatMapCompletable(Function<? super T, ? extends CompletableSource> mapper) { | ||
return concatMapCompletable(mapper, 2); |
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.
Should not we pass 8 here since it'll get overridden to 8 in the implementation?
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.
Leave it for now.
Added to #5319 for the diagram. |
Add
concatMapCompletable()
toObservable
as discussed in #4853I didn't think it made sense in other reactive types.
Code is mostly a copy of
ObservableConcatMap
. Let me know if there is a better style of code to base this off instead. It also does not have the option to delay errors as concatMap() does, not sure if that is needed.