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

2.x: Should Maybe and Single have a corresponding Transformers? #4650

Closed
ZacSweers opened this issue Oct 1, 2016 · 3 comments
Closed

2.x: Should Maybe and Single have a corresponding Transformers? #4650

ZacSweers opened this issue Oct 1, 2016 · 3 comments

Comments

@ZacSweers
Copy link
Contributor

ZacSweers commented Oct 1, 2016

Currently, Maybe.compose() requires a Function<? super Maybe<T>, ? extends MaybeSource<R>> and Single.compose() requires something similar

While other types have FlowableTransformer and CompletableTransformer. Would it make sense to bring them into the same pattern? Aside from consistency, early testing for me shows that the generics gymnastics involved in Maybe and Single's Function approaches is a major headache, as any parameterized stream type gets lost and requires manual specification, forfeiting the ability to reuse APIs.

Consider RxLifecycle, which historically has just returned an implementation of the required Transformer.

someSingle
    .compose(RxLifecycle.bind(this).forSingle())
    .subscribe();

This works for simple unparameterized types, but breaks down when parameterized types come in.

screenshot 2016-10-01 02 21 10

// Same result with both Single and Maybe. Screenshot is Maybe, example below is Single.
// The type is now a List<T> of some type T
someListSingle
    .compose(Confine.to(this).forSingle())  // Compiler error because R type instance not found
    .subscribe();

Where Confine.to returns a bridging helper like this:

public static class LifecycleTransformer2<T> implements FlowableTransformer<T, T> {

  // Delegate transformer, what's currently returned by RxLifecycle.bind()
  private LifecycleTransformer<T> delegate;

  public static <T> LifecycleTransformer2<T> create(@NonNull LifecycleTransformer<T> delegate) {
    return new LifecycleTransformer2<>(delegate);
  }

  private LifecycleTransformer2(@NonNull LifecycleTransformer<T> delegate) {
    this.delegate = delegate;
  }

  public Function<Maybe<T>, MaybeSource<T>> forMaybe() {
    return source -> {
      Observable<T> o = toV1Observable(source.toFlowable());
      o = delegate.call(o);
      return RxJavaInterop.toV2Flowable(o).singleElement();
    };
  }

  public Function<? super io.reactivex.Single<T>, ? extends SingleSource<T>> forSingle() {
    return new Function<io.reactivex.Single<T>, SingleSource<T>>() {
      @Override
      public SingleSource<T> apply(io.reactivex.Single<T> source) throws Exception {
        Single<T> o = toV1Single(source);
        o = (Single<T>) delegate.forSingle().call((Single<Object>) o);
        return RxJavaInterop.toV2Single(o);
      }
    };
  }

  public CompletableTransformer forCompletable() {
    return source -> {
      Completable o = toV1Completable(source);
      o = delegate.forCompletable().call(o);
      return RxJavaInterop.toV2Completable(o);
    };
  }

  @Override
  public Publisher<? extends T> apply(Flowable<T> source) throws Exception {
    Observable<T> o = toV1Observable(source);
    o = delegate.call(o);
    return RxJavaInterop.toV2Flowable(o);
  }
}

FlowableTransformer and CompletableTransformer work fine, but the other two have been really tricky to nail down.

@akarnokd
Copy link
Member

akarnokd commented Oct 1, 2016

We have those kind of transformers but they are not wired up to compose for some reason (forgotten?). PR welcome.

@ZacSweers
Copy link
Contributor Author

Totally missed that they already existed. Done! - #4651

akarnokd pushed a commit that referenced this issue Oct 1, 2016
* Switch Maybe and Single to use their Transformers in compose()

Resolves #4650

* Update compose() tests
@akarnokd
Copy link
Member

akarnokd commented Oct 1, 2016

Closing via #4651

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants