Use the F# TIE fighter operator combination to eliminate a hard-to-name intermediary value.

A doctrine of Clean Code is the Boy Scout Rule: leave the code cleaner than you found it. Attempting to live by that rule, I'm always looking for ways to improve my code.

Writing properties with FsCheck is enjoyable, but I've been struggling with expressing ad-hoc Arbitraries in a clean style. Although I've already twice written about this, I've recently found another improvement.

Cleaner, but not clean enough #

Previously, I've described how you can use the backward pipe operator to avoid enclosing a multi-line expression in brackets:

[<Property(QuietOnSuccess = true)>]
let ``Any live cell with > 3 live neighbors dies`` (cell : int * int) =
    let nc = Gen.elements [4..8] |> Arb.fromGen
    Prop.forAll nc <| fun neighborCount ->
        let liveNeighbors =
            cell
            |> findNeighbors
            |> shuffle
            |> Seq.take neighborCount
            |> Seq.toList
        
        let actual : State =
            calculateNextState (cell :: liveNeighbors |> shuffle |> set) cell
 
        Dead =! actual

This property uses the custom Arbitrary nc to express a property about Conway's Game of Life. The value nc is a Arbitrary<int>, which is a generator of integer values between 4 and 8 (both included).

There's a problem with that code, though: I don't care about nc; I care about the values it creates. That's the important value in my property, and this is the reason I reserved the descriptive name neighborCount for that role. The property cares about the neighbour count: for any neighbour count in the range 4-8, it should hold that blah blah blah, and so on...

Unfortunately, I was still left with the need to pass an Arbitrary<'a> to Prop.forAll, so I named it the best I could: nc (short for Neighbour Count). Following Clean Code's heuristics for short symbol scope, I considered it an acceptable name, albeit a bit cryptic.

TIE Fighters to the rescue! #

One day, I was looking at a property like the one above, and I thought: if you consider the expression Prop.forAll nc in isolation, you could also write it nc |> Prop.forAll. Does it work in this context? Yes it does:

[<Property(QuietOnSuccess = true)>]
let ``Any live cell with > 3 live neighbors dies`` (cell : int * int) =
    let nc = Gen.elements [4..8] |> Arb.fromGen
    (nc |> Prop.forAll) <| fun neighborCount ->
        let liveNeighbors =
            cell
            |> findNeighbors
            |> shuffle
            |> Seq.take neighborCount
            |> Seq.toList
        
        let actual : State =
            calculateNextState (cell :: liveNeighbors |> shuffle |> set) cell
 
        Dead =! actual

This code compiles, and is equivalent to the first example. I deliberately put the expression nc |> Prop.forAll in brackets to be sure that I'd made a correct replacement. Pipes are left-associative, though, so in this case, the brackets are redundant:

[<Property(QuietOnSuccess = true)>]
let ``Any live cell with > 3 live neighbors dies`` (cell : int * int) =
    let nc = Gen.elements [4..8] |> Arb.fromGen
    nc |> Prop.forAll <| fun neighborCount ->
        let liveNeighbors =
            cell
            |> findNeighbors
            |> shuffle
            |> Seq.take neighborCount
            |> Seq.toList
        
        let actual : State =
            calculateNextState (cell :: liveNeighbors |> shuffle |> set) cell
 
        Dead =! actual

This still works (and fails if the system under test has a defect).

Due to referential transparency, though, the value nc is equal to the expression Gen.elements [4..8] |> Arb.fromGen, so you can replace it:

[<Property(QuietOnSuccess = true)>]
let ``Any live cell with > 3 live neighbors dies`` (cell : int * int) =
    Gen.elements [4..8]
    |> Arb.fromGen
    |> Prop.forAll <| fun neighborCount ->
        let liveNeighbors =
            cell
            |> findNeighbors
            |> shuffle
            |> Seq.take neighborCount
            |> Seq.toList
        
        let actual : State =
            calculateNextState (cell :: liveNeighbors |> shuffle |> set) cell
 
        Dead =! actual

In the above example, I've also slightly reformatted the expression, so that each expression composed with a forward pipe is on a separate line. That's not required, but I find it more readable.

Notice how Prop.forAll is now surrounded by pipes: |> Prop.forAll <|. This is humorously called TIE fighter infix.

Summary #

Sometimes, giving an intermediary value a descriptive name can improve code readability, but in other cases, such a value is only in the way. This was the case with the nc value in the first example above. Using TIE fighter infix notation enables you to get rid of a redundant, hard-to-name symbol. In my opinion, because nc didn't add any information to the code, I find the refactored version easier to read. I've left the code base cleaner than I found it.



Wish to comment?

You can add a comment to this post by sending me a pull request. Alternatively, you can discuss this post on Twitter or somewhere else with a permalink. Ping me with the link, and I may respond.

Published

Tuesday, 17 May 2016 10:55:00 UTC

Tags



"Our team wholeheartedly endorses Mark. His expert service provides tremendous value."
Hire me!
Published: Tuesday, 17 May 2016 10:55:00 UTC