CODEWITHSUNDEEP
f#functional-programming
fjdumont
fjdumont

Reputation: 1537

FP - Condense and 'nice' code

Writing code in F# in most cases results in very condense an intuitive work. This piece of code looks somehow imperative and inconvenient to me.

Lines inside the file times.csv always look like that:

Mai 06 2011 05:43:45 nachm.,00:22.99
Mai 04 2011 08:59:12 nachm.,00:22.73
Mai 04 2011 08:58:27 nachm.,00:19.38
Mai 04 2011 08:57:54 nachm.,00:18.00

let times =
    File.ReadAllLines "times.csv"
    |> Array.map (fun l -> float (l.Substring((l.LastIndexOf ':') + 1)))
let average set =
    (Array.sum set - Array.min set - Array.max set) / float (set.Length - 2)
let getAllSubsetsOfLengthN n (set:float list) =
    seq { for i in [0 .. set.Length - n] -> set
                                            |> Seq.skip i
                                            |> Seq.take n }
let bestAverageOfN n =
    times
    |> Array.toList
    |> getAllSubsetsOfLengthN n
    |> Seq.map (fun t -> t
                         |> Seq.toArray
                         |> average)
    |> Seq.min

What I am looking for are nicer, shorter or easier solutions. Every useful post will be upvoted, of course :)

Upvotes: 1

Views: 214

Answers (4)

Stephen Swensen
Stephen Swensen

Reputation: 22307

Since you mentioned regex for parsing your input, I thought I'd show you such a solution. It may well be overkill, but it is also a more functional solution since regular expressions are declarative while substring stuff is more imperative. Regex is also nice since it is easier to grow if the structure of your input changes, index substring stuff can get messy, and I try to avoid it completely.

First a couple active patterns,

open System.Text.RegularExpressions
let (|Groups|_|) pattern input =
    let m = Regex.Match(input, pattern)
    if m.Success then
        Some([for g in m.Groups -> g.Value] |> List.tail)
    else
        None

open System
let (|Float|_|) input =
    match Double.TryParse(input) with
    | true, value -> Some(value)
    | _ -> None

Adopting @ildjarn's times implementation:

let times =
    File.ReadAllLines "times.csv"
    |> Array.map (function Groups @",.*?:(.*)$" [Float(value)] -> value)

Upvotes: 2

ildjarn
ildjarn

Reputation: 62995

Since bestAversageOfN has already been covered, here's an alternative implementation of times:

let times =
    File.ReadAllLines "times.csv"
    |> Array.map (fun l -> l.LastIndexOf ':' |> (+) 1 |> l.Substring |> float)

Upvotes: 1

Tomas Petricek
Tomas Petricek

Reputation: 243116

Without much thinking, there are some basic functional refactorings you can make. For example, in the calculation of bestAverageOfN, you can use function composition:

let bestAverageOfN n =
    times
    |> Array.toList
    |> getAllSubsetsOfLengthN n
    |> Seq.map (Seq.toArray >> average)
    |> Seq.min

Other than this and the suggestion by desco, I don't think there is anything I would change. If you don't use your special average function anywhere in the code, you could write it inline as a lambda function, but that really depends on your personal preferences.

Just for the sake of generality, I would probably make times an argument of bestAverageOfN:

let bestAverageOfN n times =
    times
    |> Seq.windowed n
    |> Seq.map (fun set ->
           (Array.sum set - Array.min set - Array.max set) / float (set.Length - 2))
    |> Seq.min

Upvotes: 3

desco
desco

Reputation: 16782

I guess, getAllSubsetsOfLengthN can be replaced with Seq.windowed

so bestAverageOfN will look like:

let bestAverageOfN n =
    times
    |> Seq.windowed n
    |> Seq.map average
    |> Seq.min

Upvotes: 4

Related Questions