Implement MergeSources and BindReturn #4
Conversation
There was a problem hiding this comment.
the implementations are sound, but this won't result in and! making async calls parallel (if that's the intention in mediaingenuity/OpenBanking.AffordabilityInsights#12)
or is it still for the calling client to co-ordinate the parallelism, then zip together the responses?
| let zip left right = | ||
| match left, right with | ||
| | Ok x1res, Ok x2res -> Ok(x1res, x2res) | ||
| | Error e, _ -> Error e | ||
| | _, Error e -> Error e |
There was a problem hiding this comment.
comparing with FsToolKit Result.zip
seems fine but looks like they have a typo in the comment: // Ok (Some(1, 2)) ?
worth a PR there, too!
There was a problem hiding this comment.
src/AsyncWriterResult/Library.fs
Outdated
| let zip left right = | ||
| bind (fun l -> bind (fun r -> retn (l, r)) right) left |
There was a problem hiding this comment.
personally think the CE is more readable in this case rather than nested inline functions. Tho maybe not so straight forward in the 'deeper' types? logaf low.
| let zip left right = | |
| bind (fun l -> bind (fun r -> retn (l, r)) right) left | |
| let zip left right = | |
| async { | |
| let! l = left | |
| let! r = right | |
| return (l, r) | |
| } |
There was a problem hiding this comment.
Yeah I was thinking to keep it using bind just because the zip definition should be the same in any context so if we need to implement a zip in another CE we can just copy paste this line. I've updated it to work in parallel now thought based on Don Syme's recommendation in this comment dotnet/fsharp#11043 (comment)
| @@ -262,6 +288,8 @@ type AsyncWriterResultBuilder() = | |||
| member __.ReturnFrom(m: Async<Writer<'w, Result<'a, 'b>>>) = m | |||
| member __.Bind(m, f) = AsyncWriterResult.bind f m | |||
| member __.Zero() = __.Return() | |||
| member __.BindReturn(x, f) = AsyncWriterResult.map f x | |||
There was a problem hiding this comment.
didn't realise BindReturn is just map!
is it possible to use map "on its own" in a CE? as in, does the ! syntax support it?
There was a problem hiding this comment.
Sorry not quite sure what you mean by this?
There was a problem hiding this comment.
yea ignore me 😅
I'm imagining a world where let! = works on a function that just maps, but I guess that's just let =
This will allow us to use
and!