Grotesque F# Code - I

Recently a friend came to me in a mild panic about some massive refactoring he needed to do to an F# code base. The code he had was very complicated and maintenance was a pain. After only a few seconds scanning through the code I certainly could see that the code was more complicated than it needed to be, in fact I would even go so far as to say it was grotesque.

Odd or unnatural in shape, appearance, or character; fantastically ugly or absurd; bizarre.

Note that I didn’t say terrible or wrong. The code he showed me did exactly what it was supposed to do. But it was non-idomatic; so any seasoned F# developer would probably cringe when looking at it. The point of this blog post isn’t to criticize any F# code out in the wild, but rather just show how using functional programming can simplify your code.

Here is one of the most grievous sections of the code he showed me. Essentially it takes a list of data and produces a new list after ‘processing’ each element. (Obviously with the method names and parameters renamed.)

 // The original code
let computeStuff x y z = (* ... *) ()

let rec analyzeStuff paramX paramY listOfData =
    match listOfData with
    | [] 
        -> []
    | hd :: tl 
        -> 
            [ 
                computeStuff paramX paramY hd 
            ] @ analyzeStuff paramX paramY tl

 

Eliminating Unnecessary Parameters

The first thing to point out is that the function’s parameters paramX and paramY are being passed directly to the computeStuff function and otherwise aren’t even used by this function. So why keep them around? Having additional function parameters only makes it a little more confusing. Using function currying you can eliminate those parameters by simply passing in a function that only accepts one argument. (And currying the rest as necessary.)

You can rewrite the analyzeStuff method like follows. The analyzeStuff2 function only takes two parameters now, f – the function for processing – and the list of data.

 // Eliminating unnecessary parameters
let rec analyzeStuff2 f listOfData =
    match listOfData with
    | [] 
        -> []
    | hd :: tl 
        -> 
            [ 
                f hd 
            ] @ analyzeStuff2 f tl

And then, simply pass in a curried version of computeStuff. This allows you to reuse the analyzeStuff method and perhaps operate on different functions (rather than hard coding it to call computeStuff).

 // Pass in a curried function
analyzeStuff2 (computeStuff paramX paramY)

The fact that there were unnecessary parameters to the function isn’t what, in my mind, made the code so bad. There were a couple of other idiomatic F# things that stuck out…

Prefer Cons instead of Append

As I’ve mentioned in a previous post, cons (::) is fast and append (@) is slow. I mean, if Dustin Campbell has posted about it is common knowledge, right? ;)

Think for a moment about what the following code actually does:

 [ f hd ] @ analyzeStuff2 f tl

If your answer was ‘more than it needs to’ you would be correct.

1. Create a new, one element list via ‘[ f hd ]’
2. Make a copy of that entire list via ‘ @ ‘
3. Set the last element’s next pointer to the result of ‘analyzeStuff2 f tl’ (via the rest of the append)

The problem lies in bullet number 2. You create a list only to create a second, identical list so you can use append on it. If you want to put exactly one element in the front of the rest of the list use cons. Which brings us to the following code snippet, which is slightly more performant than analyzeStuff2.

 // Eliminating unnecessary append
let rec analyzeStuff3 f listOfData =
    match listOfData with
    | [] 
        -> []
    | hd :: tl 
        -> (f hd) :: analyzeStuff3 f tl

So while we have trimmed off a few characters and eliminated a few unnecessary memory allocations, there is still one final, significant problem with this code.

Using the F# Library Where Possible

Remember the whole purpose of the analyzeStuff method? It was to take a list of data and produce a new list of data after ‘processing’ each element. In other words, mapping a function to each element of the input list. This function may sound familiar – it is already built into the F# library and is called List.map. So we can remove the analyzeStuff function entirely, in lieu of just using what is already baked into the F# library.

 // Final code
let results = List.map (computeStuff paramX paramY) listOfData

The moral of the story is that if your code seems more complicated than it needs to be, then it probably is. However, once my book comes out there will be no excuse for writing grotesque F# code in the future ;)

Comments

• Anonymous
  September 13, 2009
  Chris, what is your opinion of let rec analyzeStuff3 f listOfData =    match listOfData with versus let rec analyzeStuff3 f =    function

• Anonymous
  September 13, 2009
  I can feel the beauty of funtionnal programming in this starting from the 'ugly' code till the use of List.map but I can't convince my friends about it, since they see imperative style as the best way out there, functional programming is 'odd'. how to convince them

• Anonymous
  September 14, 2009
  The comment has been removed

• Anonymous
  September 14, 2009
  The comment has been removed

• Anonymous
  September 14, 2009
  I noticed that the list they were building in the end was a list of (). Perhaps they should have used iter instead? Props to them for having code that worked although not idiomatic. I assume we'll see a lot of this as F# starts to take off.   Good post!

• Anonymous
  September 15, 2009
  @martani: > how to convince them I wish we knew :(

• Anonymous
  September 17, 2009
  Correction: analyzeStuff2 (computeStuff paramX paramY) analyzeStuff2 (computeStuff paramX paramY) listOfData

• Anonymous
  September 28, 2009
  One should be careful about rewriting the recursive form into the List.map form, as they are not strictly equivalent:

1. The recursive form may run out of stack space, whereas the List.map should not. Nothing to worry about though, unless the intent of the code is to run out of stack space ;)
2. The recursive form specifies the order in which items are processed, I don't think List.map does, or does it? If f has side-effects, then this is an important consideration.