# Parsing sections from a log file using F#

After reading a lot about F#, I am today cutting my very first real life F# code and I'd appreciate any feedback on my approach.

I'm writing a simple utility to analyse a log file. The code below represents my approach for identifying sections of the log and representing them in a structure. The "real life" situation is to find application sessions in the log file. The example below model this by parsing a list of numeric strings!

The main areas of relevance are the GetSection and GetSections methods below. My questions are:

• Is my approach sensible?
• Is this functional and idiomatic?
• Is there an alternative approach that would be clearer or more elegant?

//  Here are the "lines" from my "log file".
let lines = [1 .. 25] |> List.map (fun x -> x.ToString())

// For this exercise, section criteria is divisibility by 5.
// val DivisibleByFive : s:string -> bool
let DivisibleByFive (s:string) =
System.Int32.Parse(s) % 5 = 0

// I am going to use the following type to hold the information
// from each section of the log.
type Section (lines:string list) =
member x.Lines = lines

// Read a single section from the top of the log (based on: http://fssnip.net/iV).
// val GetSection : lines:string list -> Section * string list
let GetSection (lines:string list) =
let rec getSection (l:string list) result =
match l with
| h::t when DivisibleByFive h ->
let values = h::result |> List.rev
(new Section(values), t)
| h::t when not (DivisibleByFive h)  ->
getSection t (h::result)
| _ ->
(new Section(result), [])
getSection lines []

// Get a list of all sections from the log.
// val GetSections : lines:string list -> Section list
let GetSections (lines:string list) =
let rec getSections (l:string list) result =
match GetSection l with
| (a, []) -> (a::result)
| (a, b) -> getSections b (a::result)
getSections lines [] |> List.rev

[<EntryPoint>]
let main argv =
let sections = GetSections lines
sprintf "Parsed %d section from the log:" sections.Length |> System.Console.WriteLine
for i in sections do
sprintf "  Section starting with %s" i.Lines.[0] |> System.Console.WriteLine
0


Edit: The refactored code following the comments by @mjolka is below (including the bug fix mentioned in @mjolka's update). The refactored code is available as a LinqPad or a Gist.

//  Here are the "lines" from my "log file".
let lines = [1 .. 25] |> List.map (fun x -> x.ToString())

// For this exercise, section criteria is divisibility by 5.
// val DivisibleByFive : s:string -> bool
let DivisibleByFive (s:string) =
System.Int32.Parse(s) % 5 = 0

// I am going to use the following type to hold the information
// from each section of the log.
type Section (lines:string list) =
member x.Lines = lines

/// <summary>
/// Splits the given sequence into non-empty contiguous subsequences, such
/// that the last element of every subsequence (except possibly the last)
/// satisfies the given predicate. No other elements satisfy the predicate.
/// </summary>
/// <example>
/// <c>
/// let even x = x % 2 = 0
/// printfn "%A" (splitAtEach even (seq { 1 .. 3 }))
/// printfn "%A" (splitAtEach even (seq { 1 .. 4 }))
/// </c>
/// The output is:
/// <c>
/// seq [[1; 2]; [3]]
/// seq [[1; 2]; [3; 4]]
/// </c>
/// </example>
let splitAtEach (predicate : 'T -> bool) (xs : seq<'T>) : seq<'T list> =
let section = new ResizeArray<'T>()
seq {
for x in xs do
if predicate x then
yield Seq.toList section
section.Clear()
if section.Any() then
yield Seq.toList section
}

[<EntryPoint>]
let main argv =
let sections =
splitAtEach DivisibleByFive lines
|> Seq.map (fun lines -> new Section(lines))
|> Seq.toList
printfn "Parsed %d section from the log:" sections.Length
sections |> List.iter (fun section -> printfn "  Section starting with %s"
0


Welcome to the world of F#! Your code is looking good, and thanks for posting the LinqPad file. Here are a couple of pointers:

• sprintf ... |> System.Console.WriteLine can be replaced by a call to printfn.
• The variable name i in the for loop is misleading, as it looks like an index. Calling it section would be better.
• If you don't need the result of an expression, such as System.Console.ReadLine, it's common to name the variable _.

Then your main function would look like this:

let main argv =
let sections = GetSections lines
printfn "Parsed %d section from the log:" sections.Length
for section in sections do
printfn "  Section starting with %s" section.Lines.Head
0


You could also write the for loop using List.iter, but it's a matter of personal preference:

sections |> List.iter (fun section -> printfn "  Section starting with %s" section.Lines.Head)


Now to the main part of your code. I would say that it's functional and idiomatic, but in my experience with F#, sometimes it's best to compromise on the functional approach.

For example, consider GetSections. The basic idea is to go through a sequence, building up lists until a certain predicate is met. I feel that the functional approach, with its list reversal, obscures the intention of the code. Here is what I would propose:

Edit Fixed a bug where last elements might not be returned.

/// <summary>
/// Splits the given sequence into non-empty contiguous subsequences, such
/// that the last element of every subsequence (except possibly the last)
/// satisfies the given predicate. No other elements satisfy the predicate.
/// </summary>
/// <example>
/// <c>
/// let even x = x % 2 = 0
/// printfn "%A" (splitAtEach even (seq { 1 .. 3 }))
/// printfn "%A" (splitAtEach even (seq { 1 .. 4 }))
/// </c>
/// The output is:
/// <c>
/// seq [[1; 2]; [3]]
/// seq [[1; 2]; [3; 4]]
/// </c>
/// </example>
let splitAtEach (predicate : 'T -> bool) (xs : seq<'T>) : seq<'T list> =
let section = new ResizeArray<'T>()
seq {
for x in xs do
if predicate x then
yield Seq.toList section
section.Clear()
if section.Any() then
yield Seq.toList section
}


Then GetSections could be written

let GetSections lines =
splitAtEach DivisibleByFive lines
|> Seq.map (fun lines -> new Section(lines))
|> Seq.toList


Edit Found a bug in your definition of GetSections, where GetSections ["1"; "2"; "3"] = [["3"; "2"; "1"]].

• another common thing is doing something like System.Console.ReadLine() |> ignore without ever using let (yeah it reads a bit strange) – Carsten Jun 29 '14 at 13:44
• @mjolka - thank you, that's really useful and has helped me a lot. I agree that GetSections and GetSection were not expressing their intent as clearly as they should have been. That's a valuable lesson! :) Using Seq is much clearer and I can get a whole lot of reuse from splitAtEach in the "real" code. Very much appreciated! An updated LinqPad version is here: share.linqpad.net/as8ivp.linq. – Sean Kearon Jun 29 '14 at 15:59
• I've also now updated the question to show the refactored code. – Sean Kearon Jun 29 '14 at 16:12
• @SeanKearon I've fixed a bug in my code, please see the edit. – mjolka Jun 29 '14 at 23:10
• Thank you - I've updated the finished code above and in the LinqPad sample. – Sean Kearon Jun 30 '14 at 6:09