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 FlatMapStrategy.throttle #713

Merged
merged 9 commits into from
Nov 25, 2019

Conversation

inamiy
Copy link
Contributor

@inamiy inamiy commented Mar 6, 2019

This PR adds FlatMapStrategy.first that works as a lightweight ReactiveSwift.Action and is equivalent to RxSwift.flatMapFirst.

This PR is related to FlatMapStrategy.race #233 which works quite differently as follows:

(Marble diagram, `|` as completed)

// stream of producers
S : --P1--P2--P3--------|

// producers
P1:   --1--2-|
P2:       ----3---4-|
P3:           --5---6-|

// flatten(.merge)
FM: ----1--2--3-5-4-6---|

// flatten(.first)
FF: ----1--2----5---6---|

// flatten(.race)
FR: ----1--2------------|

As we can see in flatten(.first) that only P2 is not handled due to P1 still active (first inner producer) but P3 is well handled, this new strategy works as a lightweight Action:

let inputSignal: Signal<Input, NoError> = ...
let exec: (Input) -> SignalProducer<Output, NoError> = ...

let outputSignal = inputSignal.flatMap(.first, exec)

// ...is essentially equal to:

let action = Action(execute: exec)
inputSignal.observeValues(action.apply)

let outputSignal = action.values

Please note that if returning producer of exec can emit error, inputSignal.flatMap(.first, exec) will terminate but action.values will not.
(But I think it's also possible to fully simulate action.errors as well, by using materialize / dematerialize to pattern-match and keep the stream alive)

Checklist

  • Updated CHANGELOG.md.

@inamiy
Copy link
Contributor Author

inamiy commented Mar 10, 2019

I'm still wondering if .first is really a proper term to use for this operator since it makes us confuse a lot with .race and also SignalProducer.first.

Any thoughts on this?

@inamiy
Copy link
Contributor Author

inamiy commented Mar 10, 2019

This PR is ready to review, but requires write permission to move on ("Ready for review" button doesn't appear in my screen):

https://github.blog/changelog/2019-03-08-expand-ready-for-review-permissions-for-draft-pull-requests/

@inamiy inamiy marked this pull request as ready for review March 15, 2019 04:37
@leonid-s-usov
Copy link
Contributor

maybe this should be called .oldest vs the existing .latest?

@andersio
Copy link
Member

andersio commented Sep 8, 2019

@leonid-s-usov This is not exactly "oldest", in the sense that it only drops upstream values when the inner producer is still active. If we have to coin an accurate term in Swift collection terms, it would be like skipWhenActive.

@andersio
Copy link
Member

I would propose to name this throttle given throttle(_:on:) having a similar semantic but instead in the time domain.

CHANGELOG.md Outdated Show resolved Hide resolved
@inamiy inamiy changed the title Add FlatMapStrategy.first Add FlatMapStrategy.throttle Nov 21, 2019
@inamiy
Copy link
Contributor Author

inamiy commented Nov 21, 2019

Renamed to FlattenStrategy.throttle in 1f241b6 .

@inamiy inamiy requested a review from andersio November 21, 2019 00:03
@andersio andersio merged commit 21a1e99 into ReactiveCocoa:master Nov 25, 2019
@inamiy inamiy deleted the FlattenStrategy.first branch November 26, 2019 13:11
@mluisbrown
Copy link
Contributor

mluisbrown commented Nov 27, 2019

As we can see in flatten(.first) that only P2 is not handled due to P1 still active (first inner producer) but P3 is well handled

This makes no sense to me at all. P1 completes before P2 has emitted any values, so P1 still active is not true.

Also, since P1 completes before P2 has emitted any values, P2 should take precedence, so I would expect the output to be:

FF: ----1--2--3-5---6---|

@@ -96,6 +97,21 @@ public struct FlattenStrategy {
/// Any failure from the inner streams is propagated immediately to the flattened
/// stream of values.
public static let race = FlattenStrategy(kind: .race)

/// Forward only events from the "first inner stream" that sends an event if not exists.
Copy link
Contributor

Choose a reason for hiding this comment

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

the "first inner stream" that sends an event if not exists.

What does that mean? If what "not exists" (doesn't exist) ?

/// first inner stream again.
///
/// The flattened stream of values completes only when the stream of streams has completed,
/// and first inner stream has completed if exists.
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, don't understand if exists here.

@@ -96,6 +97,21 @@ public struct FlattenStrategy {
/// Any failure from the inner streams is propagated immediately to the flattened
/// stream of values.
public static let race = FlattenStrategy(kind: .race)

/// Forward only events from the "first inner stream" that sends an event if not exists.
/// Other inner streams is disposed of until the first inner stream is completed.
Copy link
Contributor

Choose a reason for hiding this comment

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

streams is disposed of

streams are disposed of

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

Successfully merging this pull request may close these issues.

None yet

5 participants