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 ;)