Return to Answer

• 1

• 380
• 1
• 7

I'll start with the code and explain after

let foldScript =
 let isScriptNonEmpty { Script = script } = script <> ""
 let foldAppScripts =
 let foldScripts = Seq.fold (fun acc { Script = script } -> sprintf "%s\r\n%s" acc script) ""
 Seq.groupBy (fun { App = app } -> app)
 >> Seq.map (fun (app, group) -> sprintf "\r\n\tuse Application = %s %s" app <| foldScripts group)
 >> Seq.fold (sprintf "%s%s") ""
 Seq.filter isScriptNonEmpty
 >> Seq.groupBy (fun { Store = store } -> store)
 >> Seq.map (fun (store, group) -> sprintf "use Store=%s%s" store <| foldAppScripts group)
 >> String.concat "\r\n"
let s = [ { Store = "s1"; App = "a1"; Script = "script1" }; { Store = "s1"; App = "a2"; Script = "script2" }; { Store = "s2"; App = "a3"; Script = "script4" }]
let r = foldScript s

• First I made inner "function" to ease readability and reused the Script type Reed talked about because it's a good idea

• Then you can pattern match (deconstruct) on record almost anywhere so
  (if there are multiple record type with same field name you'll have to qualify it but that's not the case here)

• The foldAppScripts "function" have itself an inner "function"

• I choose to favor function composition (and so made "function value") if it doesn't suit you, you can put back the argument and pipe over it

• I used the backward pipe to avoid using parenthesis (also a matter of taste)

• The last Seq.fold of foldAppScripts use eta-reduction (transforming fun arg -> fct arg in just fct)

• Your script.ToString () isn't needed as script is already a string

• I replace your last fold by a call to String.concat

Performance wise you could try to use a StringBuilder (and use Printf.bprintf) but it's not so easy to use

We could go further and remove each lambda by a named function (self documentation) but that's maybe overkill.

Edit

After toying a little more with this I think I managed to make the StringBuilder version working.
Not sure it was worth the effort my (simplistic) measurements show a quicker first version (some ms for 100k items so negligible) but the second has a little less pressure on GC gen 1
But that was mainly for the experiment (I finally used Printf.kbprintf)

here is the code :

let foldScript storeAppScripts =
 let builder = System.Text.StringBuilder (60 * Seq.length storeAppScripts)
 let bprintf format = Printf.bprintf builder format
 let kbprintf continuation format = Printf.kbprintf continuation builder format
 let isScriptNonEmpty { Script = script } = script <> ""
 let writeAppScripts storeAppScripts () =
 let writeScripts scripts () = Seq.iter (fun { Script = script } -> bprintf "\r\n%s" script) scripts
 storeAppScripts
 |> Seq.groupBy (fun { App = app } -> app)
 |> Seq.iter (fun (app, group) -> kbprintf (writeScripts group) "\r\n\tuse Application = %s " app)
 bprintf "\r\n"
 storeAppScripts
 |> Seq.filter isScriptNonEmpty
 |> Seq.groupBy (fun { Store = store } -> store)
 |> Seq.iter (fun (store, group) -> kbprintf (writeAppScripts group) "use Store=%s" store)
string <| builder.Remove (builder.Length - 2, 2)

• First I put back arguments because it's no more just function composition in the bodies.
• I created a StringBuilder and gave him an initial capacity
  The Seq.length means a first loop over the items so the diff in perf comes probably from there but without a proper capacity it'll suffer from many internal buffer resizing
• Then I made some helper for bprintf and kbprintf embedding the builder
  eta-reduction isn't possible for bprintf because it would then be a value and not a function so it wouldn't be generic
• I renamed the inner function to reflect the change in code they now return unit The extra unit parameter isn't strictly required it just allow me to do eta-reduction at call site as kbprintf continuation argument expect a unit -> 'a
• The rest of the code is nearly the same ; some Seq.map changed in Seq.iter because of the unit return.
  The Seq.fold vanished which probably means I could simplify the Seq.map (...) >> Seq.fold in just a fold in the first version
• At the end I remove the trailing "\r\n" added by the last call to writeAppScripts ; I wanted to find a better way without luck.

Now a little explanation about how bprintf and kbprint works :

• bprintf is "easy" it's a printf which write it's content into the Stringbuilder given (same principle as fprintf with a file)
• kbprintf do the same but take a continuation (a function) which will be executed after (for example to transform the result into another object)
  I could have not used it and just put multiple bprintf instead
  So for example kbprintf (writeAppScripts group) "use Store=%s" store will first write "use Store=XXX" into the StringBuilder and then call writeAppScripts on the group value which will add it's content into the StringBuilder