Add materialize and dematerialize for Single

[M0rtyMerr]

Working with Single events instead of elements might be important in some cases. For instance with RxSwiftExt .elements() and .errros() operators. Using single.asObervable().materialize() looks strange for me. It can be cutted to simple single.materialize().
I can't see any reasons why this operator is not added to PrimitiveSequence.

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

[M0rtyMerr]

Hey @freak4pc could u please share your thoughts about extending PrimitiveSequence with this operator? Should I add dematerialize here or in separate PR?

_{Sent with GitHawk}

[freak4pc]

If this is accepted, you'll want both. The if question is for Krunoslav, not myself.

[kzaher]

Hi @MortyMerr ,

The author suggested this example as a use case:

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

I'm confused by the implementation.

public func materialize() -> Single<Event<Element>> {

If we want to be as type safe as possible I think that the interface would be

public func materialize() -> Single<SingleEvent<Element>> {

I don't see any benefit of converting SingleEvent, MaybeEvent, CompletedEvent to Event. If we are doing that just because operators called elements() and error() exist, I would say that is not a good reason. On the other hand without having them, it is hard to justify adding materialize to Single.

I'm now thinking that we might want to change typealias SingleEvent<Element> = Result<Element>.

I think that dematerialize would only make sense if people used something like:

request.materialize().mapEventSomehow.dematerialize()

... but I guess that would be weird to me. Single already has "monadish" behavior. I'm not sure what would be the benefit of doing that.
If somebody has some good examples why they are doing that. I'm listening.

I think that using Rx in this way

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

is not the best idea:

• If the source depends on some state and this is one path how the values get the the UI (if the request succeeds or errors out), there might be some other path that controls the values that should be bound while the request is on its way. That means that there is probably some sharing operator in source. The ordering of observers in sharing operators is arbitrary. If the request finishes immediately (like read from cache) the UI values could always be set to loading.
• This either ignores the loading state or makes the loading state encoded in the Rx operator hierarchy which IMHO is not the best solution.

And yes, we have these kinds of examples in our repo also, but to me they are more of an operator playground than advice how to ideally design systems.

[M0rtyMerr]

Hi @kzaher
Thank you for comments :)
What do you think about public func materialize() -> Observable<Event<Element>> for Single/Maybe/Completable? Would it be more useful and logical than Single return type?
dematerialize can be deleted from this PR

is not the best idea:

It was just an example. "There might be" is a different question :) If there is a need of handling loading state - this code will be changed.
I find elements() and errors() quite useful. IMO It's equal with #729 approach. I just use Event instead of the custom Result type.

[freak4pc]

What do you think about public func materialize() -> Observable<Event> for Single/Maybe/Completable? Would it be more useful and logical than Single return type?

This won't make much sense because you'll lose the type, even when using dematerialize to "go back" (e.g Single -> Observable but no Observable to Single)

I think having materialize is worth-while for these traits (especially stuff like Single that's heavily used for network requests). I don't specifically think it's weird to use the general Event instead of SingleEvent etc, as Event as a superset still describes all the possible variations (as long as it's a Single<Event<Element>>)

I would prefer having SingleEvent etc but I don't find it too critical for the amount of crud we'll have to add. I obviously don't want to merge anything without Krunoslav's consent, so I'd say we still need to keep the discussion going to make some sort of decision. @kzaher WDYT?

[juanjo-ramos]

After reading the conversation in this PR and the linked issue, I don't understand, from a conceptual point of view, why Single wouldn't have materialize implemented. If it is a code smell or a bad architectural decision, I don't see it why.

I'm just curious how do people handle errors for Single. I do find myself using single.asObservable().materialize() quite frequently. If others do too, what is missing from this PR to move forward? Is it the implementation details?

[M0rtyMerr]

I am ready to change implementation in any moment, I am really interested in it. The main question is about a return type from single.materialize(). What should it be? How not to lose single semantic?

_{Sent with GitHawk}

[M0rtyMerr]

So @freak4pc what do you think? Is it possible to change implantation somehow to move forward with this PR?

_{Sent with GitHawk}

[freak4pc]

Sorry for all the time it took to answer, I've been super busy.

I think having a materialize from Single<Element> to Single<Result<Element, Error>> makes sense (especially since Result<Element, Error> is now SingleEvent). I'm not sure if dematerialize makes sense here but I'm open to it just for completeness.

I'm not sure if you're still involved in adding this @MortyMerr.

[M0rtyMerr]

Hey @freak4pc thank you for your answer and sorry for late response. I removed all changes except materialize/dematerialize for Single. Is there anything else I can do to merge this one?

[M0rtyMerr]

@freak4pc I am not sure why CI is failing. Local tests passed successfully.

[M0rtyMerr]

Hey @freak4pc why was this closed?

[freak4pc]

Just because we got rid of develop branch :(
It doesn't let us change the new base branch when doing this. Can you open this same PR on main, please? I think we can dd it to be honest. I'm not a huge user of this feature but happy to add it.