search

ReactiveX / RxGroovy

RxJava bindings for Groovy
Apache License 2.0
158 stars 25 forks source link

RxGroovy for RxJava 1.1.X #13

Open maltalex opened 8 years ago

maltalex commented 8 years ago

Are there plans to release RxGroovy for 1.1.0/1.1.1 ? It's is currently stuck at 1.0.15 from October 15.

sfitts commented 8 years ago

Continuing a thread from ReactiveX/RxJava#3998

@akarnokd - do you need someone to help maintain this? While parallel releases are obviously possible, it is a lot nicer when everything is clean. Espeically given the usage of these in upstream projects like Vertx -- it will be difficult, if not impossible for me to convince that project to use a forked release for their dependencies. Anyway, just a thought. A quick review/build of the code seems to show that is "just works", though there is a bit about a custom create signature that might be able to be removed at this point (though I don't think there is any harm in keeping it).

sfitts commented 8 years ago

FWIW, since Groovy 2.2.x most of what is done with this library isn't needed anymore (at least AFAICT). Groovy supports implicit coversion of closures to SAM types and that works transparently with all cases here except for FuncN. In that case the issue is that groovy treats the args as an array and can't figure out that it can splat them into the varargs. So in that case we will need the extra meta-method to do the work.

akarnokd commented 8 years ago

I don't know Groovy much so can't really help with coding in this project. I assume bumping to RxJava 1.1.5 is a straightforward diff though.

sfitts commented 8 years ago

I don't profess to be a Groovy internals expert, but I know enough to be dangerous. This code basically uses Groovy meta-programming to dynamically override specific methods so it can convert Groovy closures into the various functional interfaces used in RxJava.

I think there are 2 approaches here. The first is to do as you suggest and just bump the RxJava dependency to 1.1.+. This is trivial and works fine. However, it does mean that you are doing some work that isn't really needed (with at least some runtime cost).

You can remove this by narrowing the cases where you override the methods. On Groovy 2.4.x (and I think earlier versions as well) the only case that isn't handled properly is the conversion from a closure to the interface FuncN. Here we still need the overriden meta-method to execute the closure properly. So if you don't mind supporting only more modern versions of Groovy (2.4 goes back to Jan 15, 2.2 to Nov 13) then you can simplify this quite a bit.

Let me know which you prefer and I'll create the appropriate PR (though as you point out, the PR for the former will be a 1 line change in build.gradle).