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

The once FlattenStrategy. #516

Closed
wants to merge 1 commit into from
Closed

The once FlattenStrategy. #516

wants to merge 1 commit into from

Conversation

andersio
Copy link
Member

@andersio andersio commented Sep 11, 2017

Related: #423

This is a convenience that has been using in our codebase for a while, implemented as take(first: 1).race(). There are a lot of circumstances that we need only one value from a continuous producer (e.g. property).

For example, we can fold this:

property.producer
    .take(first: 1)
    // Since we have just one value, any strategy is irrelevant.
    .flatMap(.latest) { value in
        // Spawn a network request from the value.
    }

into:

property.producer
    .flatMap(.once) { value in
        // Spawn one network request from the first value received.
    }

While it doesn't add substantial functionality, it offers a sensible choice for flattening single-value stream of streams, that also reduces verbosity, from an API consumer POV.

Checklist

  • Updated CHANGELOG.md.

@@ -164,6 +181,9 @@ extension Signal where Value: SignalProducerConvertible, Error == NoError, Value

case .race:
return self.race()

case .once:
return self.take(first: 1).race()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the description you explain it's equivalent to flatMap(.latest), but internally you are using race. Which one is true?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there's only one signal, all the strategies are functionally equivalent. The only difference is the performance characteristics.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not have a case for performance here yet, just the room for optimization.

Since there's only one signal, all the strategies are functionally equivalent.

race propagates interrupted from the inner stream to the flattened stream. The other strategies do not.

Copy link
Member Author

@andersio andersio Sep 12, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, so the strategy is more or less relevant. race gives the closest semantic to then (which is essentially flatMapCompleted).

@mdiep
Copy link
Contributor

mdiep commented Sep 11, 2017

I'm pretty ambivalent about this addition. 😕

@Qata
Copy link
Contributor

Qata commented Feb 14, 2018

I'm a fan of this addition, the app I most use ReactiveSwift with has a lot of cases that'd benefit from this readability.
I've had to do a workshop to explain why all of our single value streams use .latest.

@andersio
Copy link
Member Author

andersio commented Mar 3, 2018

@mdiep I think now it has a case of performance, despite not practically relevant, in the sense that the take-once constraint enables simpler implementation. 😛

8cccb07#diff-c949eaeac664030204497a79420aaef3R866

@mdiep
Copy link
Contributor

mdiep commented Mar 5, 2018

Maybe it's time to reconsider adding a Single type of some sort. It seems like that's the real issue here: it's cumbersome to deal with a stream of n events when you know there will only be 1. #201

@andersio
Copy link
Member Author

andersio commented Mar 31, 2018

I am not quite convinced to have it as a dedicated type, since there is barely a compile time advantage AFAICT. From the static enforcement PoV, Single cannot do better than SignalProducer since it is impossible to statically guarantee an escaping closure to be invoked only once.

To facilitate it, we would also have to replicate the entire API of SignalProducer given the lack of HKT. In the end, all we gain for all these work and on going maintenance are probably just a strategy-less flatMap, perhaps a specialised input observer API that accepts value or error, and a dedicated type name that benefits from a doc PoV. Dynamic enforcement in debug builds could also be a feature.

@andersio
Copy link
Member Author

andersio commented Mar 31, 2018

a dedicated type name that benefits from a doc PoV

I guess this alone could be the primary driver though, since that's also how the division between Signal and SignalProducer started despite having minimal distinction at static time.

@mdiep
Copy link
Contributor

mdiep commented Apr 2, 2018

Yeah, I think there are benefits to documentation/reasoning. Many common tasks could be represented as a observable of one value. It would be easy to have APIs to convert between them as well.

The biggest issue is coming up with a good name.

@andersio andersio closed this Jul 28, 2018
@andersio andersio deleted the once-strategy branch July 28, 2018 13:17
@andersio andersio mentioned this pull request Aug 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants