Don't make these 5 F# mistakes
Tags: fsharp, codereview, dotnet, programming
Published: 2021-12-14 21:26:43

I'm lucky enough to be working full-time at a company that uses F#, so here are some observations I compiled from doing code reviews on real-world codebases.

## Correctness:

### 1) Prefer `string` to `.ToString()`
```fsharp
let i:int option = None
i.ToString() // throws System.NullReferenceException

// Safer ways to obtain a string representation:
string i // ""
"${i}" // ""
sprintf "%O" i // "<null>"
sprintf "%A" i // "None"
```

### 2) Don't use `failwith` or `failwithf`
It throws `System.Exception`: this violates [.NET Exception guidelines](https://docs.microsoft.com/en-us/dotnet/csharp/fundamentals/exceptions/creating-and-throwing-exceptions#things-to-avoid-when-throwing-exceptions). From experience, it's can be very annoying to debug a project where you need to break on `System.Exception`, as any random code can throw it.
Defining your own exception type in F# is a [one-liner](https://docs.microsoft.com/en-us/dotnet/fsharp/language-reference/exception-handling/exception-types), so do create your own exception types.

```fsharp
// DON'T:
failwith errorMessage

// DO
exception OrderValidationException of string
raise (OrderValidationException errorMessage)
```


## Style:
Note that the code in the following examples is perfectly fine as-is.
These are just opportunities to keep in mind to make the code easier to read.

### 3) Don't use `fst` and `snd`... 
... at least when a simple destructuring pattern would suffice. These are terribly named functions. You can't get anything but the 1st or 2nd element that way. It makes it hard to understand what was extracted unless the result is immediately bound to an identifier.

```fsharp
setupUnitTestMocks ()
|> fst  // what's this??
|> testWith
```
Prefer destructuring, with the added bonus that this will look familiar to C# and JS programmers:
```fsharp
// Ah it's a mock of a database
let databaseMock, _ =
    setupUnitTestMocks ()

testWith databaseMock
```

### 4) Don't abuse lambdas
F# makes it so easy and convenient to use pipe-to-lambda for everything that sometimes we forget we can use plain let bindings, match statements, for loops etc. Lambdas make code flow trickier to understand (what's the argument? what's being returned?). 

```fsharp
let orderInfo =
    Order.create (Guid.NewGuid().ToString())
    |> function
        | Result.Ok validatedOrder -> validatedOrder
        | _ -> raise (OrderCreationException "This should never happen")
    |> fun order ->
        { Order = order
          Description = String.Empty }
```

We can reduce indentation and make this code easier to follow with a let binding, and replacing the `function` with a `match` statement.
```fsharp
let orderInfo =
    let order =
        match Order.create (Guid.NewGuid().ToString()) with
        | Result.Ok validatedOrder -> validatedOrder
        | _ -> raise (OrderCreationException "This should never happen")

    { Order = order
      Description = String.Empty }
```


### 5) Don't systematically add parentheses around function arguments
This is a hard habit to get rid of coming from C-style languages, but function invocation is indicated by spacing, not parentheses.

Systematic use of redundant parentheses make F# code slightly heavier than it needs to be.
I want to stress that this is purely stylistic, but I suspect most F# programmers do this out of habit, not conscious thought.

```fsharp
// OK:
events.OrderReceived.Trigger(incomingOrder)

// Better:
events.OrderReceived.Trigger incomingOrder
```

This includes constructors too, which are just functions
```fsharp
let orderReceivedCompletion =
    TaskCompletionSource<_> TaskCreationOptions.RunContinuationsAsynchronously
```

_Special thanks to Don Syme for reading this article and [suggesting some improvements](https://twitter.com/dsymetweets/status/1471260583928418304)._