Skip to content

New operators: concatEmptyWith and mergeEmptyWith #3430

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

NiteshKant
Copy link
Member

As discussed in issue #3037, the primary use of these operators is to be applied to Observable<Void> so that they can be merged and concatenated with an Observable of a different type.

Both these operators raise an error if the source Observable emits any item.

As discussed in issue ReactiveX#3037, the primary use of these operators is to be applied to `Observable<Void>` so that they can be merged and concatenated with an Observable of a different type.

Both these operators raise an error if the source Observable emits any item.
@NiteshKant
Copy link
Member Author

This is a replacement for #3060

@akarnokd
Copy link
Member

akarnokd commented Oct 9, 2015

👍

@akarnokd
Copy link
Member

I've been thinking about these operators and it feels odd they have to throw if the source is not empty. Regardless, do you think if it is possible to factor out this error behavior into a separate operator, let's say none and use source.none().concatWith(other) and source.none().mergeWith(other) instead? This would match the pattern with single() which throws if there are too few or too many elements in the source.

@NiteshKant
Copy link
Member Author

The idea for these operators is to combine two streams of different types (Void and T). Since, you can not really combine two different types, it is categorized as an error, if it does so. Why do you think this is odd?

Would .none() change the type of the stream? i.e. would it be like:

public <R> Observable<R> none();

@akarnokd
Copy link
Member

It could, although that might not help due to problems with Java type inference in various Java versions (and IDEs); I got surprised a few times. Certainly, I like the inlined nature of this PR so no pressure on none. There is also the proposed Completable which doesn't allow emitting any value and supports endWith(Observable) as a means of continuation. What do you think?

@NiteshKant
Copy link
Member Author

Completable is what I was intending in a contrived way via VoidObservable in the initial issue which led to this PR. The existence of Completable will make these operators redundant and I think that is a better abstraction for this usecase.

@akarnokd
Copy link
Member

Great. (If you wish, you can try Completable without juggling with its PR.)

@abersnaze
Copy link
Contributor

Could we also close this PR if we add none() and change ignoreElements() to return <R> Observable<R>?

@akarnokd
Copy link
Member

That might not be binary compatible. I've checked it and changing the type parameter creates a compilation error in OperatorIgnoreElementsTest.testDoesNotHangAndProcessesAllUsingBackpressure, which means there could be type-expectancies out there that are require the same type as the source.

@ibaca
Copy link

ibaca commented Oct 16, 2015

I add 👍 to @akarnokd solution source.none().concatWith(other)/source.none().mergeWith(other) but changing concatWith/mergeWith by then/flat which force cast the ignoreElement (which is safe) to the other type (see #3037 (comment), sorry for commenting on the other thread, but is where @NiteshKant describes its originar problem).

@stealthcode
Copy link

Closing this PR in favor of converting an Observable to a Completable

obs.doOnNext(this::blowUp).toCompletable().endsWith(nextObservable);

Note that the operator names in Completable are subject to change as it has not yet been released. See #3570.

If you think this case is not handled then feel free to re-open.

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.

5 participants