Fsharpplus: Add result extract

[adz]

This extension returns n as the default if none: option f n
https://github.com/fsprojects/FSharpPlus/blob/master/src/FSharpPlus/Operators.fs#L39

This seems like it's equivalent to (map f >> Option.defaultValue n).

Can we add something similar for Result?

I'd like Result.defaultValue and Result.defaultWith for similarity to existing Option functions but could also be like Haskell fromLeft/fromRight... here as fromOk/fromError
https://hackage.haskell.org/package/base-4.10.0.0/docs/Data-Either.html#v:fromLeft

WDYT?

[gusty]

I think we can add the extract function, throwing a generic exception message.

We can also add a specific (maybe as an extension method) version of extract that throws (or re-throws) the Error part which is already an exception.

Then for the Haskell's fromRight, fromLeft we have to agree the names. I propose getOkWithDefault, getErrorWithDefault and getChoiceXOf2WithDefault.

More thoughts?

[wallymathieu]

Yes getOkWithDefault, getErrorWithDefault both follow name conventions so would be fine.

[adz]

I'm very :+1: on the extract. It's what motivated the issue.

However, on the other functions:

There is also Option.defaultWith which allows you to run a function rather than a value. In Option case it takes unit, since it is converting from a None.

... but that would make it getOkWithDefaultWith which reads poorly.

I think it would be more aligned to say Result.getOkDefaultValue and getOkDefaultWith along with Error versions.

The other view point is practically, when using the Result type you rarely want to 'throw away' an Ok value keeping the error case, and while it might seem more symmetrical to have both, I think having just Result.defaultValue and Result.defaultWith would be most discoverable and expected to retain the "Ok". I'd suggest aliasing these functions?

So, my suggestion:
1) getOkDefaultValue (aliased as defaultValue), getOkDefaultWith (aliased as defaultWith)
2) getErrorDefaultValue, getErrorDefaultWith

With indicates it takes a mapping function.

[gusty]

The other view point is practically, when using the Result type you rarely want to 'throw away' an Ok value keeping the error case

Do you think the same applies to Choice ?

[wallymathieu]

One of the nice things about Result type is that you might not want to throw away an Error value either, since otherwise an Option would fit better for model the result.

[gusty]

A bit off topic:

Other functions I would like to add are List.partitionOfResult and List.partitionOfChoice (also for Array and seq). See Haskell similar functions

I found them very useful in the past.

[adz]

I actually never use Choice. I'd rather have Result for errors, else how is
it different from a tuple or record? I use them instead. Only time I do use
Choice is in response from Async.Catch but I convert to Result.

On Fri, Jan 10, 2020, 12:02 AM Gustavo Leon [e-mail]
wrote:

The other view point is practically, when using the Result type you rarely
want to 'throw away' an Ok value keeping the error case

Do you think the same applies to Choice ?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/fsprojects/FSharpPlus/issues/229?email_source=notifications&email_token=AAAKBDL6NTHCRCIMOVJEAWTQ44RNTA5CNFSM4KDSLMGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIQJXWQ#issuecomment-572562394,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAKBDMV4KDLCFCSERXSY33Q44RNTANCNFSM4KDSLMGA
.

[adz]

Yes. Understand that, but I am finding that at some point in a flow of code
I want to handle some of the errors, then what to do?

At first I thought to use then the remaining errors not dealt
with should throw an exception.

Now I've settled on using (either id handle error). It's more noisy for a
ResultT of Async, but is effectively converting to plain Async...

|> ResultT.run |> map (either id handleError)

On Fri, Jan 10, 2020, 12:32 AM Oskar Gewalli [e-mail]
wrote:

One of the nice things about Result type is that you might not want to
throw away an Error value either, since otherwise an Option would fit
better for model the result.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/fsprojects/FSharpPlus/issues/229?email_source=notifications&email_token=AAAKBDKWDW77WE3ICMHANKTQ44U63A5CNFSM4KDSLMGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIQMW7Q#issuecomment-572574590,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAKBDIQKA5VF4HLK4SJGM3Q44U63ANCNFSM4KDSLMGA
.

[adz]

Yes. They have also lefts and rights. Might be rounded out with?

I have just implemented Result.partition and ResultT.parallelPartition for
when dealing with a batch of independent Async results - independent ones
where you want all not just first error.

I'd prefer them on Result and Choice.

On Fri, Jan 10, 2020, 9:54 AM Gustavo Leon [e-mail] wrote:

A bit off topic:

Other functions I would like to add are List.partitionOfResult and
List.partitionOfChoice (also for Array and seq). See Haskell similar
functions
https://hoogle.haskell.org/?hoogle=%5BEither%20A%20B%5D-%3E(%5BA%5D%2C%5BB%5D)

I found them very useful in the past.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/fsprojects/FSharpPlus/issues/229?email_source=notifications&email_token=AAAKBDICWENLKALL5MSYO3LQ46W4BA5CNFSM4KDSLMGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEISEREI#issuecomment-572803217,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAAKBDKRXYDWIFAC2EIK43DQ46W4BANCNFSM4KDSLMGA
.

[gusty]

I actually never use Choice. I'd rather have Result for errors, else how is
it different from a tuple or record?

One subtle difference is that its name implies more symmetry than Result that's why I asked.

[wallymathieu]

Yes, so it makes sense to use partition on choice then.

[gusty]

To move forward let's recap from here:

• Add extract as an extension to Result and Choice. Option has already Option.get, so we can take from there to get the names: Result.get and Choice.get or Result.getOk, Result.getError
  , Choice.get1Of2 and Choice.get2Of2. Or simply extract.

• Add the functions with defaults, names as suggested by @adz seems good to me. What about Choice?

• Add partition functions. Names I can propose: List.partitionOfResult and List.partitionOfChoice but for choice we might want to add now or in the future List.partitionOfChoice3 .. 7 . Same functions for Seq and Array.

Thoughts? PRs?

[gusty]

In #232 I did a concrete proposal of all the above.

Some notes:

It's important to note the difference between Choice<_,_> and Result<_,_> the former is like the tuple but for DUs, and it's (still) used everywhere in F# to abstract the shape of a DU while the latter is biased towards a Result-or-Error abstraction.

Therefore, some of the suggested functions makes more sense in one but not in the the other type.

In order to facilitate the use for these functions in both types we can add a simple pair of functions from one type to the other.

• These functions are Result.toChoice and Result.ofChoice it makes more sense in the Result module as Choice is more generic, so it makes no sense to add lot of conversion in Choice to other types, it's better for the other types to have their corresponding link to Choice.

• The extract function makes sense in Result which has a concrete meaning of representing a value in Ok with an effect in Error. This function is called getOk but I think it should be called simply get.

  • The additional extracting functions are also available in Result for the same reason, they are called defaultValue and defaultWith.

  • As for the partition functions, they make more sense in the Choice module as @wallymathieu suggested given the inherent symmetry and general purpose. If we want to partition on Result, we can do List.partitionMap Result.toChoice x. The name partitionMap is also used in Scala libs for the same function. There is no Seq implementation for the same reason there is no unzip for Seq which is the tricky way to implement a function that yields 2 sequences (lazy?, strict?, buffering?).

[adz]

I agree in all points except partition. Actually, I am coming around to it -- after rewriting this 3 times :)
I see that using Choice as the maps return value makes it a more generic partition.

Still, I feel that when you have a list of results, you just want to partition them, it seems less obvious to have to produce a Choice. Would you consider adding your suggestion as a built in?

let partitionResults rs = partitionMap Result.toChoice rs

Also, I assumed strict for seq, and saw it as a more generic collection. Clearly I didn't consider the potentially loss of lazy nature. I have this:

module Result
let inline partition (results : Result<'a,'b> seq) : 'a list * 'b list =
    let folder r (oks, errs) =
        match r with
        | Ok value -> (value::oks, errs)
        | Error value -> (oks,value::errs)
    foldBack folder results ([], [])

I see you've done Array and List separate implementations (I assume for performance), but I had been thinking if it was on seq it could be on Result and Choice in a more general way. Assuming so, then you're approach is the way to go.

[gusty]

Still, I feel that when you have a list of results, you just want to partition them, it seems less obvious to have to produce a Choice. Would you consider adding your suggestion as a built in?

Yes, I have the same feeling. It would be kind of choose but for Result.

Regarding the implementations I did, yes it's for performance but also for consistency (list in- list out, array in -array out).

If we want a generic implementation we can always use the one-liner:

foldMap (mapper >> function Choice1Of2 x -> [x], [] | Choice2Of2 x -> [], [x])

in any Foldable.

I prefer not to implement it for Seq at this time as I feel for functions returning 2 sequences a deep analysis/discussion needs to be done, it deserves a separate issue.

[gusty]

The additional function for partition could be called as you suggest partitionResults, or partitionOnResult but looking at other similar functions like choose and partitionMap they all take a mapper (my feeling is that choose should have been called chooseBy or chooseMap).

Another option is to add it to the Result module and call it something like Result.partitionList, so make it a very specific function, operating on a list and on an already Result (no mapper) which should work for most cases, for the rest we have the more generic version already in this PR.