Appropriate use of active patterns in F#

Question

I'm using an active pattern to parse usage events in a csv formatted usage log. The active pattern part is listed below. Parsing the whole file in works well and the sequence that is produced is filled with all sorts of UsageEvents.

type SystemName = string
type SystemVersion = string
type MAC = string
type Category = string
type Game = string
type Setting = string
type StartupLocation = string

type UsageEvent =
    | SystemStart of DateTime * SystemVersion * SystemName * MAC
    | SystemEnd of DateTime
    | GameStart of DateTime * Category * Game * Setting * StartupLocation
    | GameEnd of DateTime * Category * Game
    | Other

let (|SystemStart|SystemEnd|GameStart|GameEnd|Other|) (input : string list) =
    match List.nth input 0 with
    | "SystemStartedEvent" ->
         SystemStart (DateTime.Parse (List.nth input 1), List.nth input 2, List.nth input 3, List.nth input 4)
    | "SystemEndedEvent" ->
         SystemEnd (DateTime.Parse (List.nth input 1))
    | "GameStartedEvent" ->
         GameStart (DateTime.Parse (List.nth input 1), List.nth input 2, List.nth input 3, List.nth input 4, List.nth input 5)
    | "GameEndedEvent" ->
         GameEnd (DateTime.Parse (List.nth input 1), List.nth input 2, List.nth input 3)
    | _ ->
         Other

The problem I have is that I'm probably using the ActivePattern in the wrong way. I'd like to walk the list to create a tree out of it based on some logic, but I have no way to match an entry in the sequence after parsing.

let CountSystemStart (entries : UsageEvent list) =
    let rec loop sum = function
        | SystemStart(_,_,_,_) -> sum + 1
        | _ -> sum
    loop 0 entries

This matching does not work because the loop function required a string list. In what other way could I use the data contained in the unions or should I match the input and then store it in a regular type?

Answer 1

To add to @Petr's answer - UsageEvent cases and your active pattern cases have the same names, so the active pattern which is defined later shadows the union type. That's where the string list thing comes from, most likely.

I would just drop the active pattern altogether, and add a Parse function (or rather a ParseParts, since you want to feed it a list of strings) to the UsageEvent.

type UsageEvent =
    | SystemStart of DateTime * SystemVersion * SystemName * MAC
    | (...and so on...)
    static member ParseParts (input: string list) =
        match input with
        | ["SystemStartedEvent"; date; version; name; mac] ->
            SystemStart (DateTime.Parse date, version, name, mac)
        | (...and so on...) 

Active patterns are cute, but you really need a good scenario for them to shine. Otherwise if you can get away with using a plain function, just use a plain function.

Answer 2

There are 2 problems with this code:

1. Discriminated union UsageEvent and active pattern choice functions have the same names

2. recursive loop function is not recursive as it is written - it doesn't call itself.

When you are matching on UsageEvent list try use full type name.

I would rewrite your CountSystemStart function as:

let CountSystemStart (entries : UsageEvent list) =
    let rec loop sum = function
        | [] -> sum 
        | (UsageEvent.SystemStart(_))::rest -> loop (sum + 1) rest
        | _::rest -> loop sum rest
    loop 0 entries