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

WIP: Future and try tap each #45

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sinanspd
Copy link

Addresses #44

As originally discussed in scala/scala#8857 :

  1. Defines tapEach for Try
  2. Defines tapEach for Futures, using transform, leveraging the existing implementation for Try

Following the pattern that was laid in the previous PRs. Marking WIP because of the following mainly:

  1. Because Try is an abstract class, the pattern looks slightly different. I haven't been able to find an elegant method to define an extension method for each instance AND the abstract class, and still have things dispatch properly. I went with pattern matching to dispatch things here but it means that this extension method will need to be broken into 3 pieces, should this be ever merged into stdlib

  2. I open to suggestions for testing Futures (or testing this in general since we are testing side effects). Junit doesn't seem to be very fond of awaiting. I am looking at the Future Test in the Scala Repo and that seems to be very basic and isn't asserting any values.

Appreciate any feedback.

@sinanspd sinanspd marked this pull request as draft November 20, 2020 22:16
@NthPortal NthPortal added enhancement New feature or request library:concurrent status:pending This enhancement request lacks consensus on whether or not to include it labels Nov 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request library:concurrent status:pending This enhancement request lacks consensus on whether or not to include it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants