-
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 Maybe.flatMapSingle #4614
Conversation
|
||
final Function<? super T, ? extends SingleSource<T>> mapper; | ||
|
||
final SingleObserver<T> singleObserver = new SingleObserver<T>() { |
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.
Not sure whether this is the right approach
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 avoid anonymous inner classes such as this.
SingleSource<T> ss; | ||
|
||
try { | ||
ss = ObjectHelper.requireNonNull(mapper.apply(null), "The mapper returned a null SingleSource"); |
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.
Here is the question whether the mapper should be used or not. I thought it might make sense that Maybe.empty()
should also go through flatMapSingle
final SingleObserver<T> singleObserver = new SingleObserver<T>() { | ||
@Override | ||
public void onSubscribe(final Disposable d) { | ||
|
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.
This doesn't compose the disposable from the Single
.
|
||
final Function<? super T, ? extends SingleSource<T>> mapper; | ||
|
||
final SingleObserver<T> singleObserver = new SingleObserver<T>() { |
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 avoid anonymous inner classes such as this.
SingleSource<T> ss; | ||
|
||
try { | ||
ss = ObjectHelper.requireNonNull(mapper.apply(null), "The mapper returned a null SingleSource"); |
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.
We don't map onComplete
and mapper
shouldn't anticipate null
from the library anyaway.
Current coverage is 78.14% (diff: 85.00%)@@ 2.x #4614 diff @@
==========================================
Files 553 555 +2
Lines 36128 36202 +74
Methods 0 0
Messages 0 0
Branches 5559 5561 +2
==========================================
+ Hits 28231 28289 +58
- Misses 5891 5905 +14
- Partials 2006 2008 +2
|
@akarnokd could you check again? I think I've figured it out now how this should all work |
|
||
@Override | ||
public void onComplete() { | ||
// Ignored. |
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.
This will hang the output Single
. You should signal a NoSuchElementException
instead.
@Override | ||
protected void subscribeActual(SingleObserver<? super T> s) { | ||
FlatMapMaybeObserver<T> parent = new FlatMapMaybeObserver<T>(s, mapper); | ||
s.onSubscribe(parent); |
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.
No need to pull this out.
|
||
@Override | ||
public void onSubscribe(Disposable d) { | ||
DisposableHelper.replace(this, d); |
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.
setOnce() + actual.onSubscribe(this)
* @see <a href="http://reactivex.io/documentation/operators/flatmap.html">ReactiveX operators documentation: FlatMap</a> | ||
*/ | ||
@SchedulerSupport(SchedulerSupport.NONE) | ||
public final Single<T> flatMapSingle(final Function<? super T, ? extends SingleSource<T>> 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.
Function<? super T, ? extends SingleSource<? extends R>>
Thanks for bearing with me on this one ... :D |
Gave that implementation a try. Feedback is welcome I think there are improvements that can be done.