When variable names are in the way by Mark Seemann
While Clean Code recommends using good variable names to communicate the intent of code, sometimes, variable names can be in the way.
Good guides to more readable code, like Clean Code, explain how explicitly introducing variables with descriptive names can make the code easier to understand. There's much literature on the subject, so I'm not going to reiterate it here. It's not the topic of this article.
In the majority of cases, introducing a well-named variable will make the code more readable. There are, however, no rules without exceptions. After all, one of the hardest tasks in programming is naming things. In this article, I'll show you an example of such an exception. While the example is slightly elaborate, it's a real-world example I recently ran into.
Escaping object-orientation #
Regular readers of this blog will know that I write many RESTful APIs in F#, but using ASP.NET Web API. Since I like to write functional F#, but ASP.NET Web API is an object-oriented framework, I prefer to escape the object-oriented framework as soon as possible. (In general, it makes good architectural sense to write most of your code as framework-independent as possible.)
ASP.NET Web API expects you handle HTTP requests using Controllers, so I use Constructor Injection to inject a function that will do all the actual work, and delegate each request to a function call. It often looks like this:
type PushController (imp) = inherit ApiController () member this.Post (portalId : string, req : PushRequestDtr) : IHttpActionResult = match imp req with | Success () -> this.Ok () :> _ | Failure (ValidationFailure msg) -> this.BadRequest msg :> _ | Failure (IntegrationFailure msg) -> this.InternalServerError (InvalidOperationException msg) :> _
This particular Controller only handles HTTP POST requests, and it does it by delegating to the injected imp function and translating the return value of that function call to various HTTP responses. This enables me to compose imp from F# functions, and thereby escape the object-oriented design of ASP.NET Web API. In other words, each Controller is an Adapter over a functional implementation.
For good measure, though, this Controller implementation ought to be unit tested.
A naive unit test attempt #
Each HTTP request is handled at the boundary of the system, and the boundary of the system is always impure - even in Haskell. This is particularly clear in the case of the above PushController, because it handles Success (). In success cases, the result is () (unit), which strongly implies a side effect. Thus, a unit test ought to care not only about what imp returns, but also the input to the function.
While you could write a unit test like the following, it'd be naive.
[<Property(QuietOnSuccess = true)>] let ``Post returns correct result on validation failure`` req (NonNull msg) = let imp _ = Result.fail (ValidationFailure msg) use sut = new PushController (imp) let actual = sut.Post req test <@ actual |> convertsTo<Results.BadRequestErrorMessageResult> |> Option.map (fun r -> r.Message) |> Option.exists ((=) msg) @>
This unit test uses FsCheck's integration for xUnit.net, and Unquote for assertions. Additionally, it uses a convertsTo function that I've previously described.
The imp function for PushController must have the type PushRequestDtr -> Result<unit, BoundaryFailure>. In the unit test, it uses a wild-card (_) for the input value, so its type is 'a -> Result<'b, BoundaryFailure>. That's a wider type, but it matches the required type, so the test compiles (and passes).
FsCheck populates the req argument to the test function itself. This value is passed to sut.Post. If you look at the definition of sut.Post, you may wonder what happened to the individual (and currently unused) portalId argument. The explanation is that the Post method can be viewed as a method with two parameters, but it can also be viewed as an impure function that takes a single argument of the type string * PushRequestDtr - a tuple. In other words, the test function's req argument is a tuple. The test is not only concise, but also robust against refactorings. If you change the signature of the Post method, odds are that the test will still compile. (This is one of the many benefits of type inference.)
The problem with the test is that it doesn't verify the data flow into imp, so this version of PushController also passes the test:
type PushController (imp) = inherit ApiController () member this.Post (portalId : string, req : PushRequestDtr) : IHttpActionResult = let minimalReq = { Transaction = { Invoice = ""; Status = { Code = { Code = 0 } } } } match imp minimalReq with | Success () -> this.Ok () :> _ | Failure (ValidationFailure msg) -> this.BadRequest msg :> _ | Failure (IntegrationFailure msg) -> this.InternalServerError (InvalidOperationException msg) :> _
As the name implies, the minimalReq value is the 'smallest' value I can create of the PushRequestDtr type. As you can see, this implementation ignores the req method argument and instead passes minimalReq to imp. This is clearly wrong, but it passes the unit test test.
Data flow testing #
Not only should you care about the output of imp, but you should also care about the input. This is because imp is inherently impure, so it'd be conceivable that the input values matter in some way.
As xUnit Test Patterns explains, automated tests should contain no branching, so I don't think it's a good idea to define a test-specific imp function using conditionals. Instead, we can use guard assertions to verify that the input is as expected:
[<Property(QuietOnSuccess = true)>] let ``Post returns correct result on validation failure`` req (NonNull msg) = let imp candidate = candidate =! snd req Result.fail (ValidationFailure msg) use sut = new PushController (imp) let actual = sut.Post req test <@ actual |> convertsTo<Results.BadRequestErrorMessageResult> |> Option.map (fun r -> r.Message) |> Option.exists ((=) msg) @>
The imp function is now implemented using Unquote's custom =! operator, which means that candidate must equal req. If not, Unquote will throw an exception, and thereby fail the test.
If candidate is equal to snd req, the =! operator does nothing, enabling the imp function to return the value Result.fail (ValidationFailure msg).
This version of the test verifies the entire data flow through imp: both input and output.
There is, however, a small disadvantage to writing the imp code this way. It isn't a big issue, but it annoys me.
Here's the heart of the matter: I had to come up with a name for the local PushRequestDtr value that the =! operator evaluates against snd req. I chose to call it candidate, which may seem reasonable, but that naming strategy doesn't scale.
In order to keep the introductory example simple, I chose a Controller method that doesn't (yet) use its portalId argument, but the code base contains other Controllers, for example this one:
type IdealController (imp) = inherit ApiController () member this.Post (portalId : string, req : IDealRequestDtr) : IHttpActionResult = match imp portalId req with | Success (resp : IDealResponseDtr) -> this.Ok resp :> _ | Failure (ValidationFailure msg) -> this.BadRequest msg :> _ | Failure (IntegrationFailure msg) -> this.InternalServerError (InvalidOperationException msg) :> _
This Controller's Post method passes both portalId and req to imp. In order to perform data flow verification of that implementation, the test has to look like this:
[<Property(QuietOnSuccess = true)>] let ``Post returns correct result on success`` portalId req resp = let imp pid candidate = pid =! portalId candidate =! req Result.succeed resp use sut = new IdealController (imp) let actual = sut.Post (portalId, req) test <@ actual |> convertsTo<Results.OkNegotiatedContentResult<IDealResponseDtr>> |> Option.map (fun r -> r.Content) |> Option.exists ((=) resp) @>
This is where I began to run out of good argument names. You need names for the portalId and req arguments of imp, but you can't use those names because they're already in use. You can't even shadow the names of the outer values, because the test-specific imp function has to close over those outer values in order to compare them to their expected values.
While I decided to call the local portal ID argument pid, it's hardly helpful. Explicit arguments have become a burden rather than a help to the reader. If only we could get rid of those explicit arguments.
Point free #
Functional programming offers a well-known alternative to explicit arguments, commonly known as point-free programming. Some people find point-free style unreadable, but sometimes it can make the code more readable. Could this be the case here?
If you look at the test-specific imp functions in both of the above examples with explicit arguments, you may notice that they follow a common pattern. First they invoke one or more guard assertions, and then they return a value. You can model this with a custom operator:
// 'Guard' composition. Returns the return value if ``assert`` doesn't throw. // ('a -> unit) -> 'b -> 'a -> 'b let (>>!) ``assert`` returnValue x = ``assert`` x returnValue
The first argument, ``assert``, is a function with the type 'a -> unit. This is the assertion function: it takes any value as input, and returns unit. The implication is that it'll throw an exception if the assertion fails.
After invoking the assertion, the function returns the returnValue argument.
The reason I designed it that way is that it's composable, which you'll see in a minute. The reason I named it >>! was that I wanted some kind of arrow, and I thought that the exclamation mark relates nicely to Unquote's use of exclamation marks.
This enables you to compose the first imp example (for PushController) in point-free style:
[<Property(QuietOnSuccess = true)>] let ``Post returns correct result on validation failure`` req (NonNull msg) = let imp = ((=!) (snd req)) >>! Result.fail (ValidationFailure msg) use sut = new PushController (imp) let actual = sut.Post req test <@ actual |> convertsTo<Results.BadRequestErrorMessageResult> |> Option.map (fun r -> r.Message) |> Option.exists ((=) msg) @>
At first glance, most people would be likely to consider this to be less readable than before, and clearly, that's a valid standpoint. On the other hand, once you get used to identify the >>! operator, this becomes a concise shorthand. A data-flow-verifying imp mock function is composed of an assertion on the left-hand side of >>!, and a return value on the right-hand side.
Most importantly, those hard-to-name arguments are gone.
Still, let's dissect the expression ((=!) (snd req)) >>! Result.fail (ValidationFailure msg).
The expression on the left-hand side of the >>! operator is the assertion. It uses Unquote's must equal =! operator as a function. (In F#, infix operators are functions, and you can use them as functions by surrounding them by brackets.) While you can write an assertion as candidate =! snd req using infix notation, you can also write the same expression as a function call: (=!) (snd req) candidate. Since this is a function, it can be partially applied: (=!) (snd req); the type of that expression is PushRequestDtr -> unit, which matches the required type 'a -> unit that >>! expects from its ``assert`` argument. That explains the left-hand side of the >>! operator.
The right-hand side is easier, because that's the return value of the composed function. In this case the value is Result.fail (ValidationFailure msg).
You already know that the type of >>! is ('a -> unit) -> 'b -> 'a -> 'b. Replacing the generic type arguments with the actual types in use, 'a is PushRequestDtr and 'b is Result<'a ,BoundaryFailure>, so the type of imp is PushRequestDtr -> Result<'a ,BoundaryFailure>. When you set 'a to unit, this fits the required type of PushRequestDtr -> Result<unit, BoundaryFailure>.
This works because in its current incarnation, the imp function for PushController only takes a single value as input. Will this also work for IdealController, which passes both portalId and req to its imp function?
Currying #
The imp function for IdealController has the type string -> IDealRequestDtr -> Result<IDealResponseDtr, BoundaryFailure>. Notice that it takes two arguments instead of one. Is it possible to compose an imp function with the >>! operator?
Consider the above example that exercises the success case for IdealController. What if, instead of writing
let imp pid candidate = pid =! portalId candidate =! req Result.succeed resp
you write the following?
let imp = ((=!) req) >>! Result.succeed resp
Unfortunately, that does work, because the type of that function is string * IDealRequestDtr -> Result<IDealResponseDtr, 'a>, and not string -> IDealRequestDtr -> Result<IDealResponseDtr, BoundaryFailure>, as it should be. It's almost there, but the input values are tupled, instead of curried.
You can easily correct that with a standard curry function:
let imp = ((=!) req) >>! Result.succeed resp |> Tuple2.curry
The Tuple2.curry function takes as input a function that has tupled arguments, and turns it into a curried function. Exactly what we need here!
The entire test is now:
[<Property(QuietOnSuccess = true)>] let ``Post returns correct result on success`` req resp = let imp = ((=!) req) >>! Result.succeed resp |> Tuple2.curry use sut = new IdealController (imp) let actual = sut.Post req test <@ actual |> convertsTo<Results.OkNegotiatedContentResult<IDealResponseDtr>> |> Option.map (fun r -> r.Content) |> Option.exists ((=) resp) @>
Whether or not you find this more readable than the previous example is, as always, subjective, but I like it because it's a succinct, composable way to address data flow verification. Once you get over the initial shock of partially applying Unquote's =! operator, as well as the cryptic-looking >>! operator, you may begin to realise that the same idiom is repeated throughout. In fact, it's more than an idiom. It's an implementation of a design pattern.
Mocks #
When talking about unit testing, I prefer the vocabulary of xUnit Test Patterns, because of its unrivalled consistent terminology. Using Gerard Meszaros' nomenclature, a Test Double with built-in verification of interaction is called a Mock.
Most people (including me) dislike Mocks because they tend to lead to brittle unit tests. They tend to, but sometimes you need them. Mocks are useful when you care about side-effects.
Functional programming emphasises pure functions, which, by definition, are free of side-effects. In pure functional programming, you don't need Mocks.
Since F# is a multi-paradigmatic language, you sometimes have to write code in a more object-oriented style. In the example you've seen here, I've shown you how to unit test that Controllers correctly work as Adapters over (impure) functions. Here, Mocks are useful, even if they have no place in the rest of the code base.
Being able to express a Mock with a couple of minimal functions is, in my opinion, preferable to adding a big dependency to a 'mocking library'.
Concluding remarks #
Sometimes, explicit values and arguments are in the way. By their presence, they force you to name them. Often, naming is good, because it compels you to make tacit knowledge explicit. In rare cases, though, the important detail isn't a value, or an argument, but instead an activity. An example of this is when verifying data flow. While the values are obviously present, the focus ought to be on the comparison. Thus, by making the local function arguments implicit, you can direct the reader's attention to the interaction - in this case, Unquote's =! must equal comparison.
In the introduction to this article, I told you that the code you've seen here is a real-life example. This is true.
I submitted my refactoring to point-free style as an internal pull request on the project I'm currently working. When I did that, I was genuinely in doubt about the readability improvement this would give, so I asked my reviewers for their opinions. I was genuinely ready to accept if they wanted to reject the pull request.
My reviewers disagreed internally, ultimately had a vote, and decided to reject the pull request. I don't blame them. We had a civil discussion about the pros and cons, and while they understood the advantages, they felt that the disadvantages weighed heavier.
In their context, I understand why they decided to decline the change, but that doesn't mean that I don't find this an interesting experiment. I expect to use something like this in the future in some contexts, while in other contexts, I'll stick with the more verbose (and harder to name) test-specific functions with explicit arguments.
Still, I like to solve problems using well-known compositions, which is the reason I prefer a composable, idiomatic approach over ad-hoc code.
If you'd like to learn more about unit testing and property-based testing in F# (and C#), you can watch some of my Pluralsight courses.
