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

Possibly create a ChoiceT Monad Transformer #99

Closed
Tombert opened this issue Mar 21, 2018 · 4 comments · Fixed by #105
Closed

Possibly create a ChoiceT Monad Transformer #99

Tombert opened this issue Mar 21, 2018 · 4 comments · Fixed by #105
Assignees
Labels

Comments

@Tombert
Copy link

Tombert commented Mar 21, 2018

Description

While I do see a ResultT transformer, several .NET libraries still use the (admittedly outdated) Choice type (e.g. Async.Catch). The Choice type is incompatible with the Result type.

Repro steps

N/A

Expected behavior

N/A

Actual behavior

N/A

Known workarounds

I suppose I could create a function called choiceToResult like this

let choiceToResult myThingy =
    match myThingy with
    | Choice1Of2 x -> Ok x
    | Choice2Of2 ex -> Error ex

However, this is a bit clunky since I'd have to pipe every Choice result into this function.

@wallymathieu
Copy link
Member

There is this existing issue #52

@gusty
Copy link
Member

gusty commented Mar 22, 2018

Creating a ChoiceT may seem as easy as to copy-paste ResultT and replace Result with Choice but there are other implications: ResultT works with our MonadError abstraction, which by looking at the code should also work in theory with Choice.

We can test this approach by trying to re-write some sample code with ChoiceT

Otherwise it should be straight-forward to create an Async.Catch version that creates a Result instead of a Choice, we can call it Async.CatchToResult.

@wallymathieu
Copy link
Member

Sounds like a nice thing for a pull request 😉 @Tombert

@wallymathieu wallymathieu self-assigned this Apr 11, 2018
@wallymathieu
Copy link
Member

I will start to look into making a PR for it @Tombert . We can pair on it if you want.

wallymathieu added a commit that referenced this issue Apr 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants