Coding Practice for F#

Question

I have been dabbling with F# in Visual Studio 2010. I am a developer with more code/architecture design experience in object-oriented languages such as C# and Java.

To expand my skill set and help make better decisions I am trying different languages to do different things. In particular get the hang of coding "correctly" using functional languages (in this case F#).

A simple example is generating some XML, then adding some filters to eliminate some elements.

Here is my code:

open System
open System.Xml.Linq


let ppl:(string * string) list = [
    ("1", "Jerry"); 
    ("2", "Max"); 
    ("3", "Andrew");
]

/// Generates a Person XML Element, given a tuple.
let createPerson (id:string, name:string) = new XElement(XName.Get("Person"),
                                                new XAttribute(XName.Get("ID"), id),
                                                new XElement(XName.Get("Name"), name)
)

/// Filter People by having odd ID's
let oddFilter = fun (id:string, name:string) -> (System.Int32.Parse(id) % 2).Equals(1) 

/// Open filter which will return all people
let allFilter = fun (id:string, name:string) -> true

/// Generates a People XML Element.
let createPeople filter = new XElement(XName.Get("People"), 
                                ppl |> List.filter(filter)  |> List.map createPerson
)

/// First XML Object
let XmlA = createPeople oddFilter

/// Second XML Object
let XmlB = createPeople allFilter


printf "%A\n\n%A" XmlA XmlB


/// Waits for a keypress
let pauseKey = fun () -> System.Console.ReadKey() |> ignore


pauseKey()

My questions are: What things have I done well in this scenario? What parts could be done better?

I am really looking forward to some ideas and I am quite excited about becoming familiar with functional paradigms too! :)

Thanks in advance

Answer 1

In principle, your code is all right.

There are just some points that can be simplified from a syntactical point of view.

let ppl:(string * string) list = [
    ("1", "Jerry"); 
    ("2", "Max"); 
    ("3", "Andrew");
]

The compiler is able to deduce most types by himself:

let ppl = [ "1", "Jerry";
            "2", "Max";
            "3", "Andrew" ]

And of course you can re-write your filters like this due to currying:

let oddFilter (id:string, name:string) = (int id) % 2 = 1
let allFilter (id:string, name:string) = true

The biggest improvement would be separating the indices from the names and let the programm do the numbering. You don't have to work with strings instead of numbers and can use more idiomatic tuple-free functions:

let ppl = [ "Jerry"; "Max"; "Andrew" ]

let oddFilter id name = id % 2 = 1
let allFilter id name = true

let createPerson id name = ...

The part

ppl |> List.filter(filter)  |> List.map createPerson

would be rewritten to

[ for (index, name) in List.mapi (fun i x -> (i, x)) do
      if filter index name then
          yield createPerson (string index) name ]

Answer 2

let createPeople filter = new XElement(XName.Get("People"), 
                            ppl |> List.filter(filter)  |> List.map createPerson
)

This part can be deforested manually, or you can hope that the compiler will deforest it for you.

Basically, there is an intermediate structure (the list of filtered people) that, if this is compiled naively, will be allocated to serve only once. Better apply createPerson on each element as it is decided if they are in or out, and build the final result directly.

EDIT: cfern contributed this deforested version of createPeople:

let createPeople filter = 
  new XElement(
    XName.Get("People"), 
    List.foldBack 
      (fun P acc -> if filter P then (createPerson P)::acc else acc) 
      ppl 
      [])

NOTE: because there could be side-effects in filter or createPerson, in F# it is rather hard for the compiler to decide to deforest by itself. In this case it seems to me that deforesting is correct because even if filter has side-effects, createPerson doesn't but I'm no specialist.