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

Added spaced function, similar to metered but waiting #2684

Merged
merged 1 commit into from
Oct 27, 2021

Conversation

eugeniasimich
Copy link
Contributor

While working with the library I was kind of expecting metered to behave like the function I add in this PR.
In short, the difference between spaced and metered is analogous to the one between fixedDelay and fixedRate. I also think having both makes more clear what metered does.
Also decided not to have two versions of it (like metered and meteredStartsImmediately) but that's just an opinion.
I can add documentation as well. What do you think?

@eugeniasimich eugeniasimich changed the title Spaced Added spaced function, similar to metered but waiting Oct 20, 2021
@diesalbla
Copy link
Contributor

Also decided not to have two versions of it (like metered and meteredStartsImmediately) but that's just an opinion.

Perhaps a Boolean parameter startsImmediately with a default choice (false) would do.

I can add documentation as well. What do you think?

I can suggest some changes

Copy link
Contributor

@diesalbla diesalbla left a comment

Choose a reason for hiding this comment

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

So, while writing that documentation suggestion, it just came to me...

How would this combinator differ from just doing the following?

  stream.evalTap(_ => Temporal[F].sleep(delay))

(trying to remember the Temporal api as I can)

As described in the long comment, the purpose of metered is to make those delays aware and adjusted to other delays in the whole stream, either by the source stream or by the consumer. If this action is to be independent of that context, the evalTap could do?

core/shared/src/main/scala/fs2/Stream.scala Outdated Show resolved Hide resolved
@eugeniasimich
Copy link
Contributor Author

eugeniasimich commented Oct 22, 2021

The difference between spaced and evalTapping a delay is that, for finite streams, spacing the stream won't perform a delay after the last element.

@eugeniasimich
Copy link
Contributor Author

@diesalbla I've added a commit to address you suggestion

Comment on lines 680 to 682
* The resulting stream emits the same elements from `this` stream,
* but it emits them in singleton chunks. Between each singleton chunk
* (each element) it introduces a pause that takes the `delay` duration.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* The resulting stream emits the same elements from `this` stream,
* but it emits them in singleton chunks. Between each singleton chunk
* (each element) it introduces a pause that takes the `delay` duration.
* The resulting stream emits the same elements from `this` stream,
* but split into singleton chunks. Between each chunk (element) it
* adds a pause of a fixed `delay` duration.

(slight correction)

Copy link
Contributor

@diesalbla diesalbla left a comment

Choose a reason for hiding this comment

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

@eugeniasimich Before merging, could you squash all commits into one?

@eugeniasimich
Copy link
Contributor Author

done!

@mpilquist mpilquist merged commit 23f8499 into typelevel:main Oct 27, 2021
@eugeniasimich eugeniasimich deleted the spaced branch October 28, 2021 14:53
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.

3 participants