patternMinor
F# function to concatenate some DSL scripts with indentation
Viewed 0 times
scriptswithfunctionindentationconcatenatedslsome
Problem
I'm rather new to F# and functional programming. I wrote the following method to concatenate some custom dsl scripts. The function works are expected without errors. I wanted to know if there was a better way to rewrite this function.
let foldScript (storeAppScripts : seq) =
storeAppScripts
|> Seq.choose( fun (store, app, script) ->
match script with
| "" -> None
| _ -> Some (store, app, script) )
|> Seq.groupBy(fun (store, app, script) -> store)
|> Seq.map(fun (store, group) -> store, (group
|> Seq.groupBy(fun (_, app, _)-> app)
|> Seq.map(fun (app, group) -> app , (group |> Seq.fold(fun accum (_,_, script) -> sprintf "%s\r\n%s" accum script) ""))
|> Seq.map(fun (app, script) -> sprintf "\r\n\tuse Application = %s %s" app (script.ToString()))))
|> Seq.map(fun (store, script) ->
sprintf "use Store=%s%s" store (script |> Seq.fold( fun accum appScript -> sprintf "%s%s" accum appScript) ""))
|> Seq.fold(fun acc script -> sprintf "%s\r\n%s" acc script) ""
let s = [("s1", "a1", "script1"); ("s1", "a2", "script2"); ("s2", "a3", "script4")]
let r = foldScript s
//Expected result
// use Store=s1
// use Application = a1
// script1
// use Application = a2
// script2
// use Store=s2
// use Application = a3
// script1Solution
I'll start with the code and explain after
-
First I made inner "function" to ease readability and reused the
-
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
-
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
-
Your
-
I replace your last fold by a call to String.concat
Performance wise you could try to use a
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
here is the code :
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
eta-reduction isn't possible for
The extra unit parameter isn't strictly required it just allow me to do eta-reduction at call site as
The
Now a little explanation about how bprintf and kbprint works :
I could have not used it and just put multiple bprintf instead
So for example
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 > 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 > 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 useWe 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
bprintfandkbprintfembedding 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.mapchanged inSeq.iterbecause 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 towriteAppScripts; 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 StringBuilderCode Snippets
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 slet 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)Context
StackExchange Code Review Q#134296, answer score: 6
Revisions (0)
No revisions yet.