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

add Subscribers.wrap #3065

Merged
merged 1 commit into from
Jul 14, 2015
Merged

Conversation

davidmoten
Copy link
Collaborator

Add utility method to Subscribers to perform this common use case:

Naming briefly discussed in #3057.

new Subscriber<T>(subscriber) {

    @Override
    public void onCompleted() {
        subscriber.onCompleted();
    }

    @Override
    public void onError(Throwable e) {
        subscriber.onError(e);
    }

    @Override
    public void onNext(T t) {
        subscriber.onNext(t);
    }

};

subscriber.onCompleted();
}
});
observable.unsafeSubscribe(Subscribers.from(subscriber));
Copy link
Member

Choose a reason for hiding this comment

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

This calls from but it should be wrap.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

erk, thanks!

@benjchristensen
Copy link
Member

I'm trying to remind myself what in these cases we can't just pass the subscriber through if we're not actually changing any behavior. Unsubscription will propagate up and down, so it's not decoupling anything. So why must we allocate and wrap in these use cases?

@davidmoten
Copy link
Collaborator Author

Double call of onStart was one reason.

On Tue, 14 Jul 2015 07:08 Ben Christensen [email protected] wrote:

I'm trying to remind myself what in these cases we can't just pass the
subscriber through if we're not actually changing any behavior.
Unsubscription will propagate up and down, so it's not decoupling anything.
So why must we allocate and wrap in these use cases?


Reply to this email directly or view it on GitHub
#3065 (comment).

@davidmoten
Copy link
Collaborator Author

used wrap instead of from in OnSubscribeUsing and rebased.

@benjchristensen
Copy link
Member

Double call of onStart was one reason.

Ah right. So wrap is really about when we need to decouple or handle nested subscribes.

child.onCompleted();
}
};
return Subscribers.wrap(child);
Copy link
Member

Choose a reason for hiding this comment

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

This one is less obvious as to why it needs to be wrapped. The duplicate onStart problem seems like something we should fix inside Subscriber rather than needing to object allocate and wrap.

benjchristensen added a commit that referenced this pull request Jul 14, 2015
@benjchristensen benjchristensen merged commit bd8fb30 into ReactiveX:1.x Jul 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants