search

jack-pappas / ExtCore

An extended core library for F#.
Apache License 2.0
180 stars 32 forks source link

AsyncResult based on AsyncChoice #30

Closed wallymathieu closed 6 years ago

wallymathieu commented 6 years ago

In order to utilise the recently added type Result in Fsharp.Core it would be nice to be able to use similar compositions as has been implemented in ExtCore for Choice.

wallymathieu commented 6 years ago

However, since there are a lot of code using Choice<'ok,'err> instead of Result<'ok,'err>, the use of Result<,> could be unwieldy when using ExtCore (since you get an implicit assumption that you are using Choice<,>)

wallymathieu commented 6 years ago

One alternative that might be better, is to create a new lib, ExtCore.FsharpResult or release a new major version of ExtCore with a new API using FsharpResult instead of Choice<_,'Error>. Thoughts @jack-pappas ?

vasily-kirichenko commented 6 years ago

One alternative that might be better, is to create a new lib, ExtCore.FsharpResult or release a new major version of ExtCore with a new API using FsharpResult instead of Choice<_,'Error>.

I'm for a new major version entirely based on Result type.

wallymathieu commented 6 years ago

I have a branch where I've started working on that https://github.com/wallymathieu/ExtCore/tree/using_result_instead_of_choice

vasily-kirichenko commented 6 years ago

@wallymathieu great! It may be as simple as defining a Choice'2 type (instead of the one from FSharp.Core), renaming Choice1Of2 to Ok (using VS rename refactoring for example), Choice2Of2 to Error, Choice'2 to Result'2, and finally removing the type to make use the standard Result.

wallymathieu commented 6 years ago

I've changed all of the Choice<_, 'Error> on that branch and added some Result.ofChoice and Result.toChoice

vasily-kirichenko commented 6 years ago

Have you transformed AsyncChoice, ReaderChoice, etc. as well?

wallymathieu commented 6 years ago

Added a new pull request for it #31 @vasily-kirichenko

wallymathieu commented 6 years ago

Yes, I have

wallymathieu commented 6 years ago

Perhaps there should be Choice.toResult and Choice.ofResult as well?

vasily-kirichenko commented 6 years ago

Perhaps there should be Choice.toResult and Choice.ofResult as well?

I don't think all such functions are needed. Choice should not be used for error handling anymore. Single place where it's actually used in entire FSharp.Core is Async.Catch and it's so sad we cannot change it to use Result because of backward compatibility. However, we should add an equivalent function to ExtCore, something like AsyncResult.attempt. What do you think?

wallymathieu commented 6 years ago

I'm closing this pull request as the other one is more in line with FSharp semantics