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

rx.operators -> rx.internal.operators #1294

Merged

Conversation

benjchristensen
Copy link
Member

Move rx.operators to rx.internal.operators for clarity that these can change at any time and are not part of the public API.

Discussed at #1270 (comment)

benjchristensen added a commit that referenced this pull request May 30, 2014
@benjchristensen benjchristensen merged commit 5f4efbf into ReactiveX:master May 30, 2014
@benjchristensen benjchristensen deleted the rx-internal-operators branch May 30, 2014 19:05
@jbripley
Copy link
Contributor

Would it make sense to create a similar rx.internal.schedulers package and move out some of the re-used code from Scheduler inner classes? Like NewThreadScheduler.RxThreadFactory or NewThreadScheduler.NewThreadWorker. @akarnokd, do you have an opinion on this?

@davidmoten
Copy link
Collaborator

I'm assuming that the meaning of internal for rxjava users will be that

  • may suffer breaking changes in any version
  • not recommended for direct use

I'm not using them directly myself but it strikes me that one of the advantages of the Operator interface will be lost to users and that is composability. Do we want to take this away from users?

@benjchristensen
Copy link
Member Author

I'm assuming that the meaning of internal for rxjava users will be that ...

Those 2 bullet points are correct. The Operator itself though is not internal, it's rx.Observable.Operator so is completely available to them.

What type of composability are you concerned with? I can't think of any reason why users would need to use the Operator* classes directly.

Would it make sense to create a similar rx.internal.schedulers package and move out some of the re-used code from Scheduler inner classes

I'm open to that. Seems to make sense.

@akarnokd
Copy link
Member

Internal scheduler package is okay with me. We may move The SubjectSubscriptionManager as well and make static inner classes normal classes.

In addition, I would consider moving Schedulers static methods directly into Scheduler, Subscribers static methods into Subscriber, perhaps even Subject.newAsync() company. This should match the way Java 8 added Stream.of() instead of Streams or StreamUtils. Unfortunately, without static interface methods, Observer and Subscription can't be unified this way.

@jbripley
Copy link
Contributor

Ok, I'll make a pull request request that creates the rx.internal.schedulers package and move what I feel is appropriate there. Thinking of moving RxThreadFactory to rx.internal.util though, since it's not really Scheduler specific.

@davidmoten
Copy link
Collaborator

My issue is that if I want to create my own Operator that is the composition of other operators then in the implementation of the Operator interface I can write for instance

public class MyOperator<T> implements Operator<T> {
   @Override
  public Subscriber<? super T> call(final Subscriber<? super T> subscriber) {
      return operator1.call(operator2.call(operator3.call(subscriber)));
  }
}

If the Operator* classes are marked as internal then this implies that I should not instantiate my operator1,operator2,operator3 classes from the internal package.

The ability to be able to easily construct Operators from others is also discussed in #983.

@benjchristensen benjchristensen mentioned this pull request Jun 1, 2014
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.

None yet

4 participants