CODEWITHSUNDEEP
f#
Lammot
Lammot

Reputation: 3

How to combine (?) two Results together

I'm starting to play around with F# and challenged myself to write a FizzBuzz (ikr, dream big).

After watching a couple of Scott Wlaschin talks, I'm trying to refactor my mess by using all those cool maps and binds and etc, but I found myself being stuck on merging 2 results together.

What i have here is this function (Result<int, string> * Result<int, string>) -> Result<int list, string>:

    let rangeToList (from, ``to``) =
        match from, ``to`` with
        | Ok first, Ok last -> Ok [ first..last ]
        | Error error, Ok _ -> Error error
        | Ok _, Error error -> Error error
        | Error error1, Error error2 -> Error(error1 + "; " + error2)

Results here are the results of parsing int from string (console input).

I have a feeling, this notation can be simplified somehow, but I can't seem to figure out how. Any tips?

Upvotes: 0

Views: 647

Answers (2)

Brian Berns
Brian Berns

Reputation: 17038

Your code is fine as is, but I think the pattern you're sensing is called an "applicative". Applicatives are like a lite version of monads, so instead of Bind, the important function is called MergeSources in F#. You can use this to create a simple computation builder, like this:

type ResultBuilder() =

    member _.BindReturn(result, f) =
        result |> Result.map f

    member _.MergeSources(result1, result2) =
        match result1, result2 with
            | Ok ok1, Ok ok2 -> Ok (ok1, ok2)
            | Error error, Ok _
            | Ok _, Error error -> Error error
            | Error error1, Error error2 -> Error (error1 + "; " + error2)

let result = ResultBuilder()

And you can then use this to implement an elegant version of rangeToList with a computation expression, like this:

let rangeToList (from, ``to``) =
    result {
        let! first = from
        and! last = ``to``
        return [ first..last ]
    }

The nice thing about this approach is that it separates the general Result-wrangling logic in MergeSources from the domain-specific logic in rangeToList. This allows you to reuse the same result builder freely in other domains as well.

More details here and here.

Upvotes: 1

Tomas Petricek
Tomas Petricek

Reputation: 243041

I think you are fairly close to the simplest way of doing this. There are only a few tweaks I can think of.

First, I would really advice against using ``to`` as a variable name. It can be done, but it just looks ugly!

Second, you can reduce the number of cases to just three - if you first match the OK, Ok and Error, Error cases, you know that you now have one Error and one Ok, so you can join those using the | (or) pattern and always return the Error result:

let rangeToList (from, last) =
    match from, last with
    | Ok first, Ok last -> Ok [ first..last ]
    | Error error1, Error error2 -> Error(error1 + "; " + error2)
    | Error error, _ | _, Error error -> Error error

Third, if you are passing the two arguments to the function as a tuple, you could further condense this using the function keyword which defines a function that immediately matches on the argument (but this would not work if the arguments were space separated):

let rangeToList = function
    | Ok first, Ok last -> Ok [ first..last ]
    | Error error1, Error error2 -> Error(error1 + "; " + error2)
    | Error error, _ | _, Error error -> Error error

I think this solution works great. If you wanted something more clever using higher-order functions (but I really do not think this is needed here), you could define a zip function that takes two results and combines them - producing a pair of values and collecting all errors into a list:

module Result =
  let zip r1 r2 = 
    match r1, r2 with
    | Ok v1, Ok v2 -> Ok (v1, v2)
    | Error e1, Error e2 -> Error [e1; e2]
    | Error e, _ | _, Error e -> Error [e]

Using this as a helper, you could now rewrite your original function as:

let rangeToList2 (from, last) =
     Result.zip from last 
     |> Result.mapError (String.concat "; ")
     |> Result.map (fun (first, last) -> [ first..last ])

I think this is nice, but perhaps unnecessarily clever. It is not much shorter than your first version and it is certainly not much easier to understand.

Upvotes: 2

Related Questions