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

2.x: Single operators factored out, headers added #4045

Merged
merged 1 commit into from
Jun 20, 2016

Conversation

akarnokd
Copy link
Member

Single operators factored out and made them extend Single directly. Added missing headers.

@akarnokd akarnokd added this to the 2.0 RC 1 milestone Jun 20, 2016
@akarnokd akarnokd merged commit 95febf4 into ReactiveX:2.x Jun 20, 2016
@akarnokd akarnokd deleted the SingleFactoredOutOps branch June 20, 2016 20:29
implements SingleSubscriber<T>, Disposable {

/** */
private static final long serialVersionUID = -7012088219455310787L;
Copy link
Contributor

Choose a reason for hiding this comment

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

When extending types that are Serializable we should probably prevent their serialization altogether with:

private Object writeReplace() {
  throw new UnsupportedOperationException();
}

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know. Can you give some justification?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well it's likely to fail anyway since Consumer isn't Serializable, but it makes explicit the fact that this type isn't a type that's meant to be serialized. It's only Serializable as a side-effect of extending another type to save memory.

I just ran over the code and changed my mind because all these instances are internal and while they're exposed in the public API it's only through non-serializable interfaces (like SingleSubscriber in this case). You would have to go far out of your way to even try to serialize these instances and it's not worth wasting the methods for something I'm 99.99% sure will never happen.

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.

2 participants