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

Idiomatic Scala Adaptor #376

Merged
merged 38 commits into from
Sep 13, 2013

Conversation

samuelgruetter
Copy link
Contributor

This is a first pull request for #336 . The Scala Adaptor is still far from being finished (see TODOs in language-adaptors/rxjava-scala/TODO.md), but I think it's in a state where it can be merged in so that people can try it out and give feedback.

includes typesafe dematerialize
Compiles in eclipse, but not with gradle, because there
are package objects and packages with the same name.
...and detect another compiler bug
package object scala and package scala now can coexist
@cloudbees-pull-request-builder

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

@samuelgruetter
Copy link
Contributor Author

As far as testing is concerned: I mainly tested usability (in the file language-adaptors/rxjava-scala/src/main/scala/rx/lang/scala/examples/RxScalaDemo.scala), and not whether the operators behave correctly, because that's already covered by the tests in RxJava. Nevertheless, I also plan to add such tests in Scala.

@benjchristensen
Copy link
Member

Does this break any existing usability with the implicits that have been in place?

How does this new approach and those implicits interact, or this is a complete replacement?

@benjchristensen
Copy link
Member

... forgot to say ... awesome work! Thanks so much for your effort and submitting this!

@samuelgruetter
Copy link
Contributor Author

It's not a breaking change: The old RxImplicits are still there and usable. It's even imaginable to use both approaches in the same project, by importing one or the other. In Scala code, if you have an rx.lang.scala.Observable, you can always call .asJava to get the underlying rx.Observable. And to code in languages other than Scala, rx.lang.scala.Observable appears as rx.Observable, thanks to Scala's value classes.

@headinthebox
Copy link
Contributor

Ditto!

@benjchristensen
Copy link
Member

Since this is not breaking I will merge this and release 0.13.1 so people can start trying it out.

benjchristensen added a commit that referenced this pull request Sep 13, 2013
@benjchristensen benjchristensen merged commit df4d569 into ReactiveX:master Sep 13, 2013
@@ -0,0 +1,5 @@

rxjava-scala-java
Copy link
Member

Choose a reason for hiding this comment

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

@samuelgruetter Do you want this separate project being published to Maven Central? Does this code need to be a separate project or can it go under ./src/examples/ like each of the other language adaptors? As a full module it gets built and published.

Here are how the examples are placed for Clojure and Groovy:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that this not yet the way it should be. This rxjava-scala-java project only contains MovieLibUsage.java, which illustrates that Scala code using Scala Observables can be used from Java with Java Observables.
MovieLibUsage.java should reside somewhere in rxjava-scala/src/examples, and should be compiled by the Java compiler, but only after the Scala compiler has compiled MovieLib.scala. Since I have no experience with gradle, the only way I could solve this was to make a seperate project for MovieLibUsage.java which depends on the project containing MovieLib.scala.
Another solution that I found would be to instruct the Scala compiler to compile MovieLibUsage.java, too, but I don't like this solution, because I'd like to illustrate with this example that Java code depending on Scala libraries using Scala Observables can be built using the Java compiler having Scala-generated class files on the classpath.
I'd be very happy if you could set up the gradle build such that this works.

@mattrjacobs
Copy link
Contributor

@samuelgruetter Really nice work, the value class approach lends itself very well to the JVM-centric approach and allows for easy consumption by all other JVM languages.

I share your concern on the maintenance burden of keeping rx.Observable and rx.lang.scala.Observable in sync. It seems like some sort of codegen approach may ease the burden - do have a sense if that would be a worthwhile endeavor?

@samuelgruetter
Copy link
Contributor Author

@mattrjacobs the whole point of this Scala adaptor is that someone manually looks at each method in the Java Observable and asks "how should this be done in Scala?". There are more differences between Java and Scala than one might think, and the goal of the adapter is to expose an API which looks as Scala-idiomatic as possible. Some examples for these differences:

def ++[U >: T](that: Observable[U]): Observable[U] // instead of concat
def zip[U](that: Observable[U]): Observable[(T, U)] // instance method instead of static
def dematerialize[U](implicit evidence: T <:< Notification[U]): Observable[U] // ensures that it can only be called on Observables of Notifications
def onErrorResumeNext[U >: T](resumeFunction: Throwable => Observable[U]): Observable[U] // requires type parameter with lower bound to get covariance right
def fold[R](initialValue: R)(accumulator: (R, T) => R): Observable[R] // curried in Scala collections, so curry if also here
def sample(duration: Duration): Observable[T] // using Duration instead of (long timepan, TimeUnit duration)
def drop(n: Int): Observable[T] // called skip in Java, but drop in Scala
def zipWithIndex: Observable[(T, Int)] // there's only mapWithIndex in Java, because Java doesn't have tuples
def toSeq: Observable[Seq[T]] // corresponds to Java's toList
def switch[U](implicit evidence: Observable[T] <:< Observable[Observable[U]]): Observable[U] // ensures that it can only be called on Observables of Observables
def apply(range: Range): Observable[Int] // static from becomes apply, use Scala Range
def never: Observable[Nothing] // use Bottom type

So I would definitely not go for a codegen approach.

However, it might be cool to have a unit test in which we encode the information "which Java method corresponds to which Scala method", and using reflection, we check if there is a Scala method for each Java method. So if a method is added to the Java Observable but not to the Scala Observable, this test would fail.

rickbw pushed a commit to rickbw/RxJava that referenced this pull request Jan 9, 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

9 participants