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

Async.map #859

Open
5 tasks done
nojaf opened this issue Mar 27, 2020 · 8 comments
Open
5 tasks done

Async.map #859

nojaf opened this issue Mar 27, 2020 · 8 comments

Comments

@nojaf
Copy link

nojaf commented Mar 27, 2020

Add Async.map in the Async Module

I propose we add a map function to the Async module.
The main reason is that it is so trivial to have and it would provide parity with other modules (List, Result, ...)

The existing way of approaching this problem in F# is writing the function yourself:

module Async =
    let map f computation =
        async.Bind(computation, f >> async.Return)

Pros and Cons

The advantages of making this adjustment to F# are:

  • Newcomers are expecting this method to be there in my opinion.
  • It removes duplicate code in a lot of projects. (I have no numbers on this, call it a hunch).

The disadvantages of making this adjustment to F# are ...

  • One small extra function to maintain.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): XS

Related suggestions:

  • Add combinators in the Async module #318
    Not sure why exactly this was rejected, I can't trace the replies back anymore.
    Yes, this was rejected in the past, years later I wish to revisit this discussion. Purely for the map function that it.

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this
@Tarmil
Copy link

Tarmil commented Mar 27, 2020

The rejected uservoice shouldn't be a problem since it also contained ParallelChoice which has since been implemented (as Choice) 😄

@Happypig375
Copy link
Contributor

Duplicate of #36

@cartermp
Copy link
Member

Funnily enough there's no async module but a class with static members: https://github.com/dotnet/fsharp/blob/master/src/fsharp/FSharp.Core/async.fs#L1091

So this could be Async.Map with method arguments to fit the rest of the members. This does mean that pipeline style coding wouldn't work all that well.

Alternatively, a new module could be added similar to what we do in the F# tools for VS: https://github.com/dotnet/fsharp/blob/master/vsintegration/src/FSharp.Editor/Common/Pervasive.fs#L176

With map being the first addition. But then there's some more things to figure out about which kinds of things are great to add there.

@charlesroddie
Copy link

@Happypig375 Duplicate of #36

Not a duplicate. That one is asking for the ability to define maps in computation expression builders, which can be used to simplify the translations of some expressions. This one is asking for map to be exposed in the Async module. Same function but the uses are pretty much disjoint.

@Happypig375
Copy link
Contributor

Existing functions like bind are already exposed via the async computation builder. Therefore, it makes sense that map is exposed the same way. #36 uses async as an example to add map to. Therefore, this duplicates #36.

@nojaf
Copy link
Author

nojaf commented Mar 28, 2020

@cartermp thanks for explaining the module versus type.

Alternatively, a new module could be added

I'd be in favour of a module if that seems like a viable option.

@Frassle
Copy link

Frassle commented Jun 14, 2020

Echoing @Happypig375 I'd be happy if this was just on the builder, we use some other functions directly from the builder in our codebases. Given it will probably be there eventually for #36 maybe we could just add it early?

@gusty
Copy link

gusty commented Jun 15, 2020

With the upcoming applicative CE feature, Async.map can be the implementation of BindReturn for existing async workflows.

member _.BindReturn (x, f) = Async.map f x

The question is, would it be possible to make it effectively more optimal than the bind based version?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants