Skip to content

Integrate Maybe and Single into Observable. *** DO NOT MERGE #4481

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

Closed
wants to merge 1 commit into from

Conversation

abersnaze
Copy link
Contributor

I've been working on the this for too long in the background and want to get this out for review even though many of the units tests don't pass yet. Its a bit rough at the moment but there was a couple of things that I wanted to talk about.

  1. does it make sense to have ignoreElements where there is a toCompletable.
  2. the same goes for single when there is a toSingle.
  3. moved toFuture to Single since it seems to fit better.

@akarnokd
Copy link
Member

akarnokd commented Sep 6, 2016

Don't delete the old ops because we can macro fuse them back in such cases:
source.reduce(...).toFlowable()

@akarnokd
Copy link
Member

akarnokd commented Sep 6, 2016

I'll post a PR that incorporates some helper types and demoes what I mean by macro-fusion for these Flowable->X and Observable->X conversions.

@Mauin
Copy link
Contributor

Mauin commented Sep 6, 2016

@abersnaze Your commit somehow has me as it's author.

@akarnokd
Copy link
Member

akarnokd commented Sep 6, 2016

@Mauin yeah it's odd

@abersnaze
See #4484 about how we could do this while keeping the performance when converting back to the Flowable/Observable.

Also it would be great if you didn't mix plain addition to Maybe with the other changes in one PR.

@abersnaze
Copy link
Contributor Author

@Mauin something wires must have got crossed my last rebate. I'll. Try and fix it before it's merged.

@akarnokd would you like the additions as preliminary PR before this one?

@akarnokd
Copy link
Member

akarnokd commented Sep 6, 2016

Yes, that would be great.

@akarnokd akarnokd added this to the 2.0 RC 3 milestone Sep 6, 2016
@akarnokd
Copy link
Member

akarnokd commented Sep 6, 2016

Let me take out the Maybe additions into a separate PR.

@akarnokd
Copy link
Member

akarnokd commented Sep 6, 2016

does it make sense to have ignoreElements where there is a toCompletable
the same goes for single when there is a toSingle.

Reactor's Flux does return Mono<Void> for ignoreElements (last time I checked) and their community liked it. I think ignoreElements is more intuitive for naming the function. The question is, how the return type change will disrupt the developer's flow and how many time does he/she apply .toFlowable() (which we dutifully try to optimize) to get back to the Flowable/Observable world:

Flowable.range(1, 10)
.flatMap(v -> .saveToDb(restCall(v)).ignoreElements().toFlowable())
.blockingLast(0)

To avoid this burden, we have to provide flatMapCompletable(T -> Completable) etc in the Flowable/Observable as well, or for basically any XMap(Function) operator there is (concatMap, concatMapDelayError, flatMap, switchMap)

…cases where is it returning a 1 or 0 values.
@akarnokd
Copy link
Member

Closing as out of date and will be redone in subsequent PRs.

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

Successfully merging this pull request may close these issues.

3 participants