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

Add .materialize() for Single, Maybe, Completable #1982

Closed
M0rtyMerr opened this issue May 20, 2019 · 3 comments
Closed

Add .materialize() for Single, Maybe, Completable #1982

M0rtyMerr opened this issue May 20, 2019 · 3 comments

Comments

@M0rtyMerr
Copy link
Contributor

M0rtyMerr commented May 20, 2019

Working with sequence of events instead of elements might be important in some cases. When we want to prevent sequence from completion. Or with RxSwiftExt .elements() and .errros() operators, which is useful in error handling idiom:

let events = source.flatMap { [service] in 
  service.get().asObservable().materialize()
}
.share()
events.elements().bind(to: ...)
events.error().bind(to: ...)

Using single.asObervable().materialize() looks strange for me. It can be cutted to simple single.materialize(). Also we lose trait guarantees, because of cast to Observable.
I can't see any reasons why we can't materialize Single or Maybe.

My suggestion is to add .materialize() to Completable, Maybe and Single. Return type would be Single<Event<Element>>. And .dematerialize() for Single return type would be Maybe<T>

Here is the same issue & PR for RxJava - ReactiveX/RxJava#6278

What do you think @kzaher? Please, share your thoughts. I am ready to contribute :)
Here is a possible implementation - #1970

@kzaher
Copy link
Member

kzaher commented May 21, 2019

I'm not sure this would bring any significant benefit.

This code is more verbose

service.get().asObservable().materialize()

but provides me with more information than

service.get().materialize()

My suggestion is to add .materialize() to Completable, Maybe and Single. Return type would be Single<Event>

The return type obviously can't be Single because it doesn't contain any element except in the error case. Otherwise it's element + completed.

And .dematerialize() for Single return type would be Maybe

This is again not true. It's Observable.

I guess there is a chance we could add materialize with Observable return type. But dematerialize has no chance.

Let's wait a bit on this.

@M0rtyMerr
Copy link
Contributor Author

M0rtyMerr commented May 21, 2019

Thank you for your attention :)
But I tend to disagree.

service.get().asObservable().materialize()

doesn't do what I want. I want to materialize Single, and it materialized version should still emit only one element or error. With casting to Observable I am losing this behavior. I don't need that cast, cause it should be possible to materialize concrete trait.

The return type obviously can't be Single because it doesn't contain any element except in the error case.

It always contains the element which type is Event. It's not Observable, cause I guarantee it always emits once. Here is events of materialized traits:
Maybe -> (next|completed|error) + completed
Completable -> (completed|error) + completed
Single -> (next|error) + completed

But dematerialize has no chance.

Dematerialize Single<Event<Int> to Maybe<Int> seems logical, because Single can be (success|error). So dematerialized Single would be empty Maybe or successful Maybe.

Why do you think it should be Observable? It this is an only option - I am ready to change my implementation, however approach with Single seems to be correct from the contract and logical side

@kzaher
Copy link
Member

kzaher commented May 29, 2019

This is too confusing. Let's continue discussion on the PR.

@kzaher kzaher closed this as completed May 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants