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

Scala Adaptor: Inheritance, subscriptions and subjects #490

Closed

Conversation

samuelgruetter
Copy link
Contributor

I tried to add subscriptions and subjects using the value class trick, and came to the conclusion that it won't work. The problem is that inheritance and value classes don't work together, because value classes cannot be extended. We want Observable to be a value class, and at the same time, we want Subject to extend Observable, so that doesn't work.

This PR is very similar to Erik's code, but I added a trait

trait JavaWrapper[+W] {
  def asJava: W
}

which all classes extend. This allows us to have an asJava method everywhere (instead of asJavaSubject / asJavaObserver etc). The main challenge was to get the double inheritance Subject extends Observer and Observable working.
Now all wrappers are done the same way. For instance, Observable looks as follows:

trait Observable[+T] extends JavaWrapper[rx.Observable[_ <: T]] { 
  ... 
}

object Observable {
  private[Observable] class ObservableWrapper[+T](val asJava: rx.Observable[_ <: T]) extends Observable[T] {}

  def apply[T](asJava: rx.Observable[_ <: T]): Observable[T] = {
    new ObservableWrapper[T](asJava)
  }

  ...
}

In Scala code, to convert from Scala types to Java types, there's the asJava method, and to convert from Java types to Scala types, there's an apply method in each companion object.

When we used value classes, such conversions were not necessary in Java, because Scala types appeared as Java types for the Java compiler. Now, they become necessary, but note that this PR does not yet contain such conversions, but that should be no big problem.

@cloudbees-pull-request-builder

RxJava-pull-requests #413 SUCCESS
This pull request looks good

@benjchristensen
Copy link
Member

I'm reviewing this with Erik tomorrow and will get this or some variant of it pulled in.

@samuelgruetter
Copy link
Contributor Author

Yes, this PR is incomplete, closing.

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.

3 participants