Learning F# - Porting C# Function to F#

Question

I'm an advanced C# programmer learning F#. As an exercise I'm porting a function that calculates the check digit of a US ABA (routing) number. Here are 2 C# implementations:

int CalcCheckDigit(string rt)
{
    var s = new[]{3,7,1,3,7,1,3,7}
        .Zip(rt, (m,d) => m * int.Parse(d.ToString()))
        .Sum();

    return (int)Math.Ceiling(s / 10.0) * 10 - s;
}

int CalcCheckDigitOldSchool(string rt)
{
    int[] mults = new[]{3,7,1,3,7,1,3,7};
    int s = 0;
    for (int i = 0; i < mults.Length; i++)
    {
        int digit = int.Parse(rt[i].ToString());
        s += digit * mults[i];
    }

    double nextMultOfTen = Math.Ceiling(s / 10.0) * 10;
    return (int)nextMultOfTen - s;
}

And here's my crack at it with F#:

let calcCheckDigit rt =
    let s = 
        rt
        |> Seq.zip [3;7;1;3;7;1;3;7]
        |> Seq.map (fun (a,b) -> a * int(string b))
        |> Seq.sum |> float
    int (ceil (s / 10.0) * 10.0 - s)

How might the F# version be improved?

Answer 1

This isn't really F# specific because you could do the same thing in your C# code, but converting each character to a string so you can parse it as an integer seems inefficient to me. I'd be inclined to use Char.GetNumericValue instead.

Also, you could eliminate a step in your method chain by using Seq.sumBy instead of Seq.map + Seq.sum.

I might write your code like this:

open System

let calcCheckDigit rt =
    let s = 
        rt
        |> Seq.map (Char.GetNumericValue >> int)
        |> Seq.zip [3;7;1;3;7;1;3;7]
        |> Seq.sumBy (fun (a,b) -> a * b)
        |> float
    (s / 10.0 |> ceil) * 10.0 - s |> int

In my opinion, separating out the "convert a character to an integer" from the multiplication makes it easier to read. I'm using the function-composition operator to join together the Char.GetNumericValue and int functions in sequence, and then passing the resulting combined function into Seq.map.

On the last line, it's a little ambiguous just what value is being passed into ceil. It would be easy for someone unfamiliar with the code to assume that the result of the entire expression (s / 10.0) * 10.0 - s is being passed to ceil. I would be inclined to write that as I did above (obviously) but if you don't like that use of the pipe operator then I think this would also be a little more clear: int ((ceil (s / 10.0)) * 10.0 - s). Personally I'm not a fan of lots of nested parens, but to each their own.

(Before I edited this post, I mistakenly assumed exactly what I just mentioned, and so I wrote (s / 10.0) * 10.0 - s |> ceil |> int. Then I realized it wouldn't produce the same output as the C# version.)

Answer 2

I like Joel Mueller's answer the best, but here is my alternative solution that is similar to your second C# sample and would probably be more efficient than map*zip*sum:

let calcCheckDigit (rt : string) =
  let lookup = [| 3.;7.;1.;3.;7.;1.; 3.; 7. |]
  let s = Array.sum [| for i in 0 .. rt.Length-1 -> lookup.[i] * (float << string) rt.[i] |]
  int ((ceil (s / 10.0)) * 10.0 - s)