F# computation expressions: Can one be used to simplify this code?

Question

I have recently started using computation expressions to simplify my code. So far the only useful one for me is the MaybeBuilder, defined thusly:

type internal MaybeBuilder() =

    member this.Bind(x, f) = 
        match x with
        | None -> None
        | Some a -> f a

    member this.Return(x) = 
        Some x

    member this.ReturnFrom(x) = x

But I would like to explore other uses. One possibility is in the situation I am currently facing. I have some data supplied by a vendor that defines a symmetric matrix. To save space, only a triangular portion of the matrix is given, as the other side is just the transpose. So if I see a line in the csv as

abc, def, 123

this means that the value for row abc and column def is 123. But I will not see a line such as

def, abc, 123

because this information has already been given due to the symmetrical nature of the matrix.

I have loaded all this data in a Map<string,Map<string,float>> and I have a function that gets me the value for any entry that looks like this:

let myLookupFunction (m:Map<string,Map<string,float>>) s1 s2 =
    let try1 =
        match m.TryFind s1 with
        |Some subMap -> subMap.TryFind s2
        |_ -> None

    match try1 with
    |Some f -> f
    |_ ->
        let try2 =
            match m.TryFind s2 with
            |Some subMap -> subMap.TryFind s1
            |_ -> None
        match try2 with
        |Some f -> f
        |_ -> failwith (sprintf "Unable to locate a value between %s and %s" s1 s2)

Now that I know about computation expressions, I suspect that the match statements can be hidden. I can clean it up slightly using the MaybeBuilder like so

let myFunction2  (m:Map<string,Map<string,float>>) s1 s2 =
    let maybe = new MaybeBuilder()
    let try1 = maybe{
        let! subMap = m.TryFind s1
        return! subMap.TryFind s2
    }

    match try1 with
    |Some f -> f
    |_ -> 
        let try2 = maybe{
            let! subMap = m.TryFind s2
            return! subMap.TryFind s1
        }
        match try2 with
        |Some f -> f
        |_ -> failwith (sprintf "Unable to locate a value between %s and %s" s1 s2)

Doing so, I have gone from 4 match statements to 2. Is there a (not contrived) way of cleaning this up even further by using computation expressions?

Answer 1

First of all, creating a new MaybeBuilder every time you need it is kinda wasteful. You should do that once, preferably right next to the definition of MaybeBuilder itself, and then just use the same instance everywhere. This is how most computation builders work.

Second: you can cut down on the amount of clutter if you just define the "try" logic as a function and reuse it:

let myFunction2  (m:Map<string,Map<string,float>>) s1 s2 =
    let try' (x1, x2) = maybe{
        let! subMap = m.TryFind x1
        return! subMap.TryFind x2
    }

    match try' (s1, s2) with
    |Some f -> f
    |_ ->         
        match try' (s2, s1) with
        |Some f -> f
        |_ -> failwith (sprintf "Unable to locate a value between %s and %s" s1 s2)

Third, notice the pattern you're using: try this, if not try that, if not try another, etc. Patterns can be abstracted as functions (that's the whole gig!), so let's do that:

let orElse m f = match m with
   | Some x -> Some x
   | None -> f()

let myFunction2  (m:Map<string,Map<string,float>>) s1 s2 =
    let try' (x1, x2) = maybe{
        let! subMap = m.TryFind x1
        return! subMap.TryFind x2
    }

    let result = 
        try' (s1, s2)
        |> orElse (fun() -> try' (s2, s1))

    match result with
    |Some f -> f
    |_ -> failwith (sprintf "Unable to locate a value between %s and %s" s1 s2)

And finally, I think you're going about it the wrong way. What you really seem to be after is a dictionary with two-part symmetric key. So why not just do that?

module MyMatrix =
    type MyKey = private MyKey of string * string
    type MyMatrix = Map<MyKey, float>

    let mkMyKey s1 s2 = if s1 < s2 then MyKey (s1, s2) else MyKey (s2, s1)


let myFunction2 (m:MyMatrix.MyMatrix) s1 s2 =
    match m.TryFind (MyMatrix.mkMyKey s1 s2) with
    | Some f -> f
    | None -> failwith (sprintf "Unable to locate a value between %s and %s" s1 s2)

Here, MyKey is a type that encapsulates a pair of strings, but guarantees that those strings are "in order" - i.e. the first one is lexicographically "less" than the second one. To guarantee this, I made the constructor of the type private, and instead exposed a function mkMyKey that properly constructs the key (sometimes referred to as "smart constructor").

Now you can freely use MyKey to both construct and lookup the map. If you put in (a, b, 42), you will get out both (a, b, 42) and (b, a, 42).

Some aside: the general mistake I see in your code is failure to use abstraction. You don't have to handle every piece of the data at the lowest level. The language allows you to define higher-level concepts and then program in terms of them. Use that ability.

Answer 2

I understand this might be just a simplification for the purpose of asking the question here - but what do you actually want to do when none of the keys is found and how often do you expect that the first lookup will fails?

There are good reasons to avoid exceptions in F# - they are slower (I don't know how much exactly and it probably depends on your use case) and they are supposed to be used in "exceptional circumstances", but the language does have a nice support for them.

Using exceptions, you can write it as a pretty readable three-liner:

let myLookupFunction (m:Map<string,Map<string,float>>) s1 s2 =
  try m.[s1].[s2] with _ -> 
  try m.[s2].[s1] with _ ->
    failwith (sprintf "Unable to locate a value between %s and %s" s1 s2)

That said, I completely agree with Fyodor that it would make a lot of sense to define your own data structure for keeping the data rather than using a map of maps (with possibly switched keys).