tuxx
tuxx

Reputation: 503

Lazy compose Observables

Say I have this

Observable<A> getA() {
  return Observable.just(new A());
}

Observable<C> getC() {
  // ... some expensive call
  return Observable.just(new C());
}

Observable<B> getB() {
  return getA()
    .map(a -> {
      if (a.someCondition()) {
        final B b = new B();
        b.setSomeFields(...);
        return b;
      }
      if (a.otherCondition()) {
        final B b = new B();
        b.setOtherFields(...);
        return b;
      }
      final B b = new B(...);
      b.setC(getC().toBlocking().single());
      return b;
    });
}

where getC() does some expensive call (or has side-effects). I want to make this call and initialize B.c field only if a.someCondition() or a. otherCondition() are not met, as above.

How would I rewrite to get rid of .toBlocking()?

One way I thought about was to zip getA() and getC():

Observable<B> getB() {
  return Observable.zip(getA(), getC(), (a, c) -> Tuple.of(a, c))
    .map(tuple -> {
      final A a = tuple._1;
      final C c = tuple._2;
      if (a.someCondition()) {
        final B b = new B();
        b.setSomeFields(...);
        return b;
      }
      if (a.otherCondition()) {
        final B b = new B();
        b.setOtherFields(...);
        return b;
      }
      final B b = new B(...);
      b.setC(c);
      return b;
    });
}

but this would make the expensive call all the time. Also makes it difficult to read where there are more complex conditions or when I have more than 2 Observables to zip.

EDIT:

@ESala's answer below works, but sometimes switch from map to flatMap would require a lot of changes. Does the solution below also work?

Observable<B> getB() {
  return getA()
    .map(a -> {
      if (a.someCondition()) {
        final B b = new B();
        b.setSomeFields(...);
        return b;
      }
      if (a.otherCondition()) {
        final B b = new B();
        b.setOtherFields(...);
        return b;
      }
      final B b = new B(...);
      getC().forEach(c -> b.setC(c));
      return b;
    });
}

Upvotes: 1

Views: 198

Answers (1)

ESala
ESala

Reputation: 7058

You could use a flatMap instead of a map, this way you avoid the toBlocking and the expensive call is only done when necessary.

Example:

Observable<B> getB() {
  return getA()
    .flatMap(a -> {                  // <-- flatMap here
      if (a.someCondition()) {
        final B b = new B();
        b.setSomeFields(...);
        return Observable.just(b);   // <-- wrap in observable
      }
      if (a.otherCondition()) {
        final B b = new B();
        b.setOtherFields(...);
        return Observable.just(b);   // <-- wrap in observable
      }
      return getC().map(c -> {       // <-- expensive call executed
          B b = new B(...);          //     only when necessary
          b.setC(c);
          return b;
      });
    });
}

Update regarding the edit in the question:

I don't think switching to a flatMap would require a lot of changes. Do you have a concrete example?

About the alternative solution in your edit: it could work in this case, but I do not recommend it.

By doing this, you are creating another subscription, which is not managed by the main observable flow, and in general this should not happen. The forEach operator returns a Disposable which is left there hanging!

Some problems it can cause include: 1) if you unsubscribe the main observable, the forEach subscription would continue anyways, 2) if you specified a scheduler in the getC() method, the main observable could complete before the forEach had finished.

Upvotes: 1

Related Questions