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()
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. 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, so do create your own exception types.
// 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.
setupUnitTestMocks ()
|> fst // what's this??
|> testWith
Prefer destructuring, with the added bonus that this will look familiar to C# and JS programmers:
// 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?).
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.
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.
// OK:
events.OrderReceived.Trigger(incomingOrder)
// Better:
events.OrderReceived.Trigger incomingOrder
This includes constructors too, which are just functions
let orderReceivedCompletion =
TaskCompletionSource<_> TaskCreationOptions.RunContinuationsAsynchronously
Special thanks to Don Syme for reading this article and suggesting some improvements.
Top comments (2)
You are wrong about 3) and 4) (suggested alternatives are noisier with no added clarity). The 4th example is pure pain to watch. I hope that nobody writes code like that.
Hate to be the internet troll commenting bad about something, but there is no way around it.
3) is solved by a good tooling (try Ionide on VSCode) or a simple comment (in worse case scenario where your colleagues use notepad to read/edit and have no F# language knowledge)
In what world is 4 a bad suggestion?