Should we use <T, T> or <? super T, ? extends T>

[dlew]

From discussion in #44 and #46.

[bensandee]

I don't know if we're voting or not but all things equal at this point I would prefer to keep (what I suspect is) the more-common use case of Java working cleanly.

I'd be curious to know if anyone is able to use 0.3.1 with Java 8 + Retrolambda without adding the this.<T> prefix to the various bind calls in RxLifeCycle. If not, is there any magic setup that would work with both Kotlin and Java? Is it possible this is a Retrolambda issue?

[dlew]

We're not voting, we're discussing. :)

One other possibility is to try to figure out if we can use some halfway-solution that allows for full inference on both Kotlin and Java, e.g. <? super T, T> or <T, ? extends T>.

[bensandee]

In my case, changing my localBindToLifecycle() helper to use either of those variants, results in the compiler inferring type java.lang.Object instead of T.

  @SuppressWarnings("unchecked")
  @Nonnull
  public final <T> Observable.Transformer<T, T> localBindToLifecycle() {
    return (Observable.Transformer<T, T> )this.<T>bindToLifecycle();
  }

[dlew]

For anyone arriving here because of problems: there is no harm in remaining w/ 0.3.0 while we suss this out. There are no changes besides the method signature.

[dlew]

Some more research notes:

Java8 isn't a good solution because it requires lambdas the entire way. If you use compose(bindToLifecycle()).subscribe(n -> ...) then it works (whereas actually supplying an Action1<Type> does not). But we can't require retrolambda for people using this lib.

I'm hoping to find a kotlin expert who can tell me why <T, T> can't be inferred. But generally speaking the problem with Java is what @tbsandee found out: it infers Object instead of T.

[dlew]

I went onto the kotlin Slack and asked about it. It may be something that the kotlin compiler can fix. I've reported an issue, we'll see how it goes: https://youtrack.jetbrains.com/issue/KT-10250

In the meantime I'm heavily leaning towards reverting to <T, T> since most people will be using this library w/ Java.

[markrietveld]

I made an account on youtrack to vote for the issue.

I would recommend reverting too. I've confirmed that we kotlin users can write a small set of java forwarding methods for RxLifecycle that makes it work nicely:

public class BindLifecycle {
    public static <T> Observable.Transformer<? super T, T> bindActivity(Observable<ActivityEvent> lifecycle) {
        return RxLifecycle.bindActivity(lifecycle);
    }

    public static <T> Observable.Transformer<? super T, T> bindUntilActivityEvent(Observable<ActivityEvent> lifecycle, ActivityEvent fragmentEvent) {
        return RxLifecycle.bindUntilActivityEvent(lifecycle, fragmentEvent);
    }

    public static <T> Observable.Transformer<? super T, T> bindFragment(Observable<FragmentEvent> lifecycle) {
        return RxLifecycle.bindFragment(lifecycle);
    }

    public static <T> Observable.Transformer<? super T, T> bindUntilFragmentEvent(Observable<FragmentEvent> lifecycle, FragmentEvent fragmentEvent) {
        return RxLifecycle.bindUntilFragmentEvent(lifecycle, fragmentEvent);
    }

    public static <T> Observable.Transformer<? super T, T> bindView(View view) {
        return RxLifecycle.bindView(view);
    }

    public static <T, E> Observable.Transformer<? super T, T> bindView(Observable<? extends E> lifecycle) {
        return RxLifecycle.bindView(lifecycle);
    }
}

With that you can call

Observable.just("")
        .compose(BindLifecycle.bindUntilActivityEvent(this.lifecycle(), ActivityEvent.DESTROY))
        .doOnNext {println(it is String)}
        .subscribe()

And have it print true

[vanniktech]

+1 for reverting the change

[dalewking]

I think the best solution would be to create an rxlifecycle-kotlin library that provides nice extension functions on Observable that lets you drop the compose so that instead of writing:

myObservable.compose(bindToLifeCycle())
myObservable.compose(bindToView(v))

you can just write:

myObservable.bindToLifeCycle(this)
myObservable.bindToView(v)

Don't know if you have the Kotlin expertise to tackle that. I will try to look at it, but cannot make any promises.

[dlew]

It seems like the problem is in Kotlin itself and some bugs in its type inference. I reported on it and it was recently marked as obsolete, so I assume there has been some improvements made there: https://youtrack.jetbrains.com/issue/KT-10250

Regarding function extensions - that seems orthogonal to this problem. It seems like a neat idea though.

[dalewking]

I can report that the new beta release of Kotlin has fixed this and no longer requires explicitly specifying the type in .compose(bindToLifecycle())

[Update] Sorry premature there. It still doesn't compile without explicitly specifying the type parameter on the compose