OperatorTakeWhile #1115
OperatorTakeWhile #1115
Conversation
|
RxJava-pull-requests #1028 SUCCESS |
| if (isSelected) { | ||
| subscriber.onNext(args); | ||
| } else { | ||
| subscriber.onCompleted(); |
There was a problem hiding this comment.
You need to unsubscribe here too. In addition, I would add a done flag (similar to Take) so event delivery is strictly stopped by this operator.
There was a problem hiding this comment.
Is it necessary? onCompleted should call unsubscribe later. Since now both synchronous and synchronous Observables support to be unsubscribed, I feel we don't need a done flag.
There was a problem hiding this comment.
There are a few operators like merge and take where we choose to eagerly call unsubscribe otherwise it can end up waiting until the very final terminal state in SafeSubscriber which can be inefficient or cause memory leaks in extreme cases. In short, if the purpose of an operator is to end a subscription then it should invoke unsubscribe directly, and that's exactly what take and takeWhile are for.
See here in take: https://github.com/Netflix/RxJava/blob/master/rxjava-core/src/main/java/rx/operators/OperatorTake.java#L84
There was a problem hiding this comment.
Thanks for clarification.
|
RxJava-pull-requests #1038 SUCCESS |
Operator
TakeWhileIssue #1060