Using Result<,> instead of Choice<_,'Error>

[wallymathieu]

• Changed to Result in order to follow the FSharp semantics
• Added Result.ofChoice and Result.toChoice

[vasily-kirichenko]

This is awesome.

[wallymathieu]

I'm trying to debug what goes wrong with the Patricia hash map, but from what I can see it's not ordering itself in the same way from time to time. Sample outputs are:

"..2...3....4.1right"
"...3..2.1....4right"
".1....4...3..2right"

For the fold test there is no assumption of ordering, only that fold-apply is done.

        // reference keys
        let refMap = HashMap.ofSeq [for c in ["."; ".."; "..."; "...."] do yield (c, c.Length) ]
        let resultRefMap = refMap |> HashMap.fold  (fun x y z -> (y,z) :: x)  [("initial",83)]
        CollectionAssert.AreEquivalent([("initial",83);("....",4);("...",3);("..",2);(".",1)], resultRefMap)
        Assert.AreEqual(("initial",83), resultRefMap |> Array.ofSeq |> Array.last)

so a naive porting of that to foldback would be:

        // reference keys
        let refMap = HashMap.ofSeq [for c in ["."; ".."; "..."; "...."] do yield (c, c.Length) ]
        let resultRefMap =  HashMap.foldBack  (fun x y z -> (x,y) :: z) refMap [("initial",83)]
        CollectionAssert.AreEquivalent([("initial",83);("....",4);("...",3);("..",2);(".",1)], resultRefMap)
        Assert.AreEqual(("initial",83), resultRefMap |> Array.ofSeq |> Array.last)

that does look like it succeeds locally

[vasily-kirichenko]

@jack-pappas What do you think about this? I'm for merging it and publish as Nuget package version 1.0.0.

[vasily-kirichenko]

@jack-pappas @wallymathieu I create a vote on twitter https://twitter.com/kot_2010/status/924710700597080064, let's see what people think.

[jack-pappas]

I'll take a look over the changes. My initial thought is that this should be an addition to (instead of replacement for) the Choice<_,_>-based functions. I think it's best if ExtCore still supports (at least for a while) older versions of F# where reasonable.

Maybe what we should be doing with ExtCore is to have separate packages targeting different versions of F#? That'd let us support older versions of F# while still letting us target features available in newer versions of the language. If we did this, I think I'd still be in favor of making these changes an addition instead of a replacement -- that'd make it easier for projects using ExtCore with older versions of VS / F# to upgrade to a newer version of VS / F# (they can start using the package for the new F# version, but without having to immediately switch all of their code over to using Result<_,_>.

[jack-pappas]

@wallymathieu I'm not sure that HashMap test is sensible anyway, since any ordering is going to depend on the exact specifics of the hashing function used; the string hashing function differs between .NET versions / Mono (and I think .NET Framework even has a switch to allow the string hashes to be randomized with a per-process random seed). I'll just disable or delete the tests.

[wallymathieu]

The main con of that approach is that ExtCore is not really compatible with using Result due to for instance the active pattern for Choice. One way to mitigate that would be to create a ExtCore.Compability namespace with the Choice active pattern et.c. (so that you can import that namespace in order to still use Choice in your code).

[wallymathieu]

I could close this pull request and create a new one with both Choice and Result?

[jack-pappas]

@wallymathieu I think that’ll be the best log-term approach. I like your idea of moving the existing Result active pattern in ExtCore to a separate, optional namespace for compatibility.

[jack-pappas]

Also, I’ll disable/delete the broken HashMap unit tests tonight so they don’t keep making noise.

[wallymathieu]

@jack-pappas I've opened a third pull request, so I'm closing this one.