How to write clearer functional-style code?

Question

Still in the process of turning my code more and more functional in style as well as in look.

Here I have a function which I try to keep as generic as I can, passing a filter function and a calculation function as parameters.

let calcError filter (fcalc:'a -> float) (arr:'a array) =
        arr |> Array.filter filter
            |> Array.map fcalc
            |> Array.average

The signature is:

val calcError : filter:('a -> bool) -> fcalc:('a -> float) -> arr:'a array -> float

I believe this is quite standard, using calcError with partial applications.

However Array.average will raise exceptions is array is of size 0 or if null (which will not happen in my case).

Not a big fan of Exceptions in F#, I would prefer using either a (float output) or a Result.

I would then think of writing the code this way but I am not sure it is a proper way to do within a functional mindset (that I am trying to acquire). Any other solution, which I could probably be able to adapt for other similar issues, is of course welcome.

Thanks all

Solution I have in mind:

let calcError2 filter (fcalc:'a -> float) (arr:'a array) =
    let subarr = arr |> Array.filter filter
    match subarr.Length with
    | 0 -> Result.Error "array length 0"
    | _ -> subarr |> Array.map fcalc
                  |> Array.average
                  |> Result.Ok

Answer 1

Here is another version with a helper function.

let calcError filter (fcalc:'a -> float) (arr:'a array) =
    let safeAverage ar = if Array.isEmpty ar then None else Some(Array.average ar)
    arr |> Array.filter filter
            |> Array.map fcalc
            |> safeAverage

Moreover you can transform array to option to use it with any other unsafe array function.

let nat arr = if Array.isEmpty arr then None else Some(arr)


let calcError filter (fcalc:'a -> float) (arr:'a array) =
        arr |> Array.filter filter
                |> Array.map fcalc
                |> nat
                |> Option.bind (Some << Array.average )

Here is a more compact and efficient version using point free style

let calcError filter (fcalc:'a -> float)   =
       Option.bind (Some << (Array.averageBy fcalc)) << nat << Array.filter filter  

It took me a while to truly appreciate the value of creating lots of small functions. Hope it helps.

Answer 2

This is one way to do it:

let tryCalcError filter (fcalc:'a -> float) (arr:'a array) =
    arr |> Array.filter filter
        |> Array.map fcalc
        |> function
        | [||] -> None
        | arr  -> Array.average arr |> Some

It follows the convention of prefixing with try to indicate that the return value is an option. You can see that convention in several Seq.try... functions like tryFind, tryHead, tryLast, tryItem, tryPick.

Answer 3

Your code looks good to me. The only thing I'd do differently is that I wouldn't use match to test whether the array is empty - you are not binding any variables and you have just two cases, so you really can just use the if expression here.

Two other minor tweaks are that I'm using Array.isEmpty to see if the array is empty (this probably has no effect here, but if you were using sequences, it would be faster than checking the length) and I also use averageBy rather than map followed by average:

let calcError2 filter (fcalc:'a -> float) (arr:'a array) =
    let subarr = arr |> Array.filter filter
    if Array.isEmpty subarr then Result.Error "array length 0" 
    else subarr |> Array.averageBy fcalc |> Result.Ok