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

I have also recently needed to transform an XSL to an XML file. This is the F# I used to do this.

Has some interesting quirks with using the .net methods.

(* Transforms an XML document given an XSLT. *)

open System.IO
open System.Text
open System.Xml
open System.Xml.Xsl

let path = @"C:\\XSL\\"

let file_xml = path + "test.xml"
let file_xsl = path + "xml-to-xhtml.xsl"

(* Compile XSL file to allow transforms *)
let compile_xsl (xsl_file:string) = new XslCompiledTransform() |> (fun compiled -> compiled.Load(xsl_file); compiled)
let load_xml (xml_file:string) = new XmlDocument() |> (fun doc -> doc.Load(xml_file); doc)

(* Transform an Xml document given an XSL (compiled *)
let transform (xsl_file:string) (xml_file:string) = 
      new MemoryStream()
        |> (fun mem -> (compile_xsl xsl_file).Transform((load_xml xml_file), new XmlTextWriter(mem, Encoding.UTF8)); mem)
        |> (fun mem -> mem.Position <- (int64)0; mem.ToArray())

(* Return an Xml fo document that has been transformed *)
transform file_xsl file_xml
    |> (fun bytes -> File.WriteAllBytes(path + "out.html", bytes))

Answer 2

Most of the time deforesting without a specific reason, generally performance, is a bad idea. Which one of these do you see as easier to read and less error prone? Deforesting taken out of context just adds complexity and/or coupling to your code.

let createPeople filter ppl =
    ppl 
    |> List.mapi (fun i x -> (i, x)) 
    |> List.filter filter
    |> List.map createPerson

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

let createPeople filter ppl = 
    (ppl |> List.mapi (fun i x -> (i, x)), []) 
    ||> List.foldBack (fun P acc -> if filter P then (createPerson P)::acc else acc) 

Once you get used to the syntax function composition allows you to drop ppl.

let createPeople filter =
    List.mapi (fun i x -> (i, x)) 
    >> List.filter filter
    >> List.map createPerson

All of these use tupled data.

let filter (id, name) = 
    id % 2 = 1

let allFilter (id, name) = 
    true

let createPerson (id, name) = 
    ()

Answer 3

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.

Answer 4

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 ]