Error messages are really important, and really hard to write well. When software authors take the time to check your inputs, and share reasons why it fails, they are doing you a favour. So, writing error messages is good.
However, this error checking code can sometimes totally eclipse the intent of the code. We can describe these kinds of patterns as a “code smell”. I first heard about code smells in Jenny Bryan’s UseR! Keynote in 2018, which I highly recommend watching. The term “code smell”, comes from Martin Fowler’s book: Improving the Design of Existing Code). The definition of a code smell that Jenny gives in her talk is:
Structures in code that suggest (or scream for) refactoring
She then goes on to define refactoring as:
Making code easier to understand for the next person (which might be you), cheaper to modify by the next person who comes through, without changing behaviour.
The talk is a delight, and watching it again was really awesome. If you haven’t watched it, you should, and if you haven’t seen it for a while, it’s worth another watch.
Anyway, I digress.
I call this code smell, and “Error Handling Eclipse”:
When error checking code totally eclipses the intent of the code.1
The code smell looks like this:
vector_to_square <- function(data) {
dims <- sqrt(length(data))
squarable_length <- floor(dims) == dims
if (!squarable_length) {
cli::cli_abort(
c(
"Provided vector is not of a squarable length",
"{.var data} is of length {.num {length(data)}}",
"This cannot be represented as a square",
"Square root of {.var dim(data)} is: {.num {round(dims, 3)}}."
)
)
}
if (!is.numeric(data)) {
cli::cli_abort(
c(
"Provided vector must be {.cls numeric}, not {.cls {class(data)}}",
"We see that {.run is.numeric(data)} returns {.cls {class(data)}}"
)
)
}
matrix(
data = data,
nrow = dims,
ncol = dims,
byrow = TRUE
)
}The code smell, “error handling eclipse”, can be seen here where the long error messages in the body of a function crow the intent. I believe the refactoring move here is to wrap these error functions into a function. This reduces repetition, and makes the intent of the code clearer, since you don’t need to wade through error checking code. The tidyverse design document on error checking) mentions building these “error checking” functions when the error is used three times. I think there is more benefit to write it out every time.
What’s the solution?
This code can be improved by refactoring the errors as check_* functions, something like, check_array_is_square(x).
Show me the difference
To demonstrate this, let’s take a look again at the example code from above, a function that converts a vector into a square matrix:
vector_to_square <- function(data) {
dims <- sqrt(length(data))
squarable_length <- floor(dims) == dims
if (!squarable_length) {
cli::cli_abort(
c(
"Provided vector is not of a squarable length",
"{.var x} is of length {.num {length(data)}}",
"This cannot be represented as a square",
"Square root of {.var dim(data)} is: {.num {round(dims, 3)}}."
)
)
}
if (!is.numeric(data)) {
cli::cli_abort(
c(
"Provided vector must be {.cls numeric}, not {.cls {class(data)}}",
"We see that {.run is.numeric(data)} returns {.cls {class(data)}}"
)
)
}
matrix(
data = data,
nrow = dims,
ncol = dims,
byrow = TRUE
)
}Now let’s demonstrate its use, and what the errors look like:
vector_to_square(data = 1:4)
#> [,1] [,2]
#> [1,] 1 2
#> [2,] 3 4
vector_to_square(data = 1:5)
#> Error in `vector_to_square()`:
#> ! Provided vector is not of a squarable length
#> `x` is of length 5
#> This cannot be represented as a square
#> Square root of `dim(data)` is: 2.236.
vector_to_square(data = LETTERS[1:4])
#> Error in `vector_to_square()`:
#> ! Provided vector must be <numeric>, not <character>
#> We see that `is.numeric(data)` returns <character>
So we have some input checking code, which does two things:
- checks if the inputs can be a square matrix
- checks if inputs are not numeric
While we like these error messages, wrapping up the error messags as functions clarifies the intent of them, and avoids repitition:
check_if_squarable <- function(x) {
dims <- sqrt(length(x))
squarable_length <- floor(dims) == dims
if (!squarable_length) {
cli::cli_abort(
c(
"Provided vector is not of a squarable length",
"{.var x} is of length {.num {length(x)}}",
"This cannot be represented as a square",
"Square root of {.var dim(x)} is: {.num {round(dims, 3)}}."
)
)
}
}
check_if_not_numeric <- function(x) {
if (!is.numeric(x)) {
cli::cli_abort(
c(
"Provided vector must be {.cls numeric}, not {.cls {class(x)}}",
"We see that {.run is.numeric(x)} returns {.cls {class(x)}}"
)
)
}
}These error messages can then be put into the function like so:
vector_to_square <- function(data) {
check_if_squarable(data)
check_if_not_numeric(data)
dims <- sqrt(length(data))
matrix(
data = data,
nrow = dims,
ncol = dims,
byrow = TRUE
)
}The error checking code no longer eclipses your intent.
I think the main benefit here is improvement in intent - I don’t have to read some error checking code, I can read the function name check_if_squarable(data) - this tells me it checks if it is squareable, and check_if_not_numeric(data) does what it says on the tin. This means I don’t have to spend time reading error checking code, which can sometimes be quite complex. I can focus on those functions intention. This means I can summarise this function like so:
-
Do some checking of the inputs
-
Get the dimensions for a square matrix
-
Make the matrix, filling in by row
There are a couple of other benefits to this:
- All my checking functions can get re-used in the other work I do
- I can find common cases of checking and improve them.
- I see two checking functions, and it invites me to think about other checks that I might want to perform.
But now the error smells
Ah, but we didn’t check to see what these new errors look like! My friend Adam Gruer pointed out:
might a user get confused when then the error is reported as coming from the check_function rather than the function they called? What are your thoughts?
Indeed, the errors are now different, check it out:
vector_to_square(data = 1:4)
#> [,1] [,2]
#> [1,] 1 2
#> [2,] 3 4
vector_to_square(data = 1:5)
#> Error in `check_if_squarable()`:
#> ! Provided vector is not of a squarable length
#> `x` is of length 5
#> This cannot be represented as a square
#> Square root of `dim(x)` is: 2.236.
vector_to_square(data = LETTERS[1:4])
#> Error in `check_if_not_numeric()`:
#> ! Provided vector must be <numeric>, not <character>
#> We see that `is.numeric(x)` returns <character>
The error message is about the checking function, check_if_squarable() or check_if_not_numeric(). This could be confusing to the user as the error doesn’t appear to be coming from the vector_to_square() function.
Seeing this also made me realise I’ve seen this problem before and been unsure how to solve it. Thankfully, the rlang team has thought about this, and they have a helpful vignette, “Including function calls in error messages”.
They bring up two really great points. Both of which I hadn’t really thought about solving. I just thought this was the compromise.
The first is how to make sure the input checking function references the function it was called in. This is why we see the error being about check_if_squarable(), and not vector_to_square(), which is what we want.
This is solved by passing the check_ functions a call environment. So we add call = rlang::caller_env() as an argument:
check_if_squarable <- function(x,
# Add this
call = rlang::caller_env()) {
dims <- sqrt(length(x))
squarable_length <- floor(dims) == dims
if (!squarable_length) {
cli::cli_abort(
message = c(
"Provided vector is not of a squarable length",
"{.var x} is of length {.num {length(x)}}",
"This cannot be represented as a square",
"Square root of {.var dim(x)} is: {.num {round(dims, 3)}}."
),
# And this
call = call
)
}
}
check_if_not_numeric <- function(x,
# Add this
call = rlang::caller_env()) {
if (!is.numeric(x)) {
cli::cli_abort(
message = c(
"Provided vector must be {.cls numeric}, not {.cls {class(x)}}",
"We see that {.run is.numeric(x)} returns {.cls {class(x)}}"
),
# And this
call = call
)
}
}Let’s see what that looks like now:
vector_to_square(data = 1:4)
#> [,1] [,2]
#> [1,] 1 2
#> [2,] 3 4
vector_to_square(data = 1:5)
#> Error in `vector_to_square()`:
#> ! Provided vector is not of a squarable length
#> `x` is of length 5
#> This cannot be represented as a square
#> Square root of `dim(x)` is: 2.236.
vector_to_square(data = LETTERS[1:4])
#> Error in `vector_to_square()`:
#> ! Provided vector must be <numeric>, not <character>
#> We see that `is.numeric(x)` returns <character>
OK much better, the error now starts with: “Error in vector_to_square()”.
The second problem, which is hauntingly familiar, is to do with supplying argument names. Notice that the error message gives the error in terms of the argument x? Despite our argument to vector_to_square using the argument, data
vector_to_square(data = 1:4)
#> [,1] [,2]
#> [1,] 1 2
#> [2,] 3 4
vector_to_square(data = 1:5)
#> Error in `vector_to_square()`:
#> ! Provided vector is not of a squarable length
#> `x` is of length 5
#> This cannot be represented as a square
#> Square root of `dim(x)` is: 2.236.
vector_to_square(data = LETTERS[1:4])
#> Error in `vector_to_square()`:
#> ! Provided vector must be <numeric>, not <character>
#> We see that `is.numeric(x)` returns <character>
A small detail, perhaps? But I think it’s actually kind of a big problem! I don’t want my users to have to try and reckon/think about what x is, or have to imagine that I’ve written some fancy code checking functions. I just want them to know where the error came from. In more complex functions this could be even more confusing.
Thankfully, yet again, the rlang team has thought about this, in Input checkers and caller_arg(), they suggest adding arg = rlang::caller_arg(x) to our checking function, which helps capture the name of the argument that is passed to the check functions. Super neat.
Let’s demonstrate this - we change uses of x to using arg in the error message.
check_if_squarable <- function(x,
arg = rlang::caller_arg(x),
call = rlang::caller_env()) {
x_len <- length(x)
dims <- sqrt(x_len)
squarable_length <- floor(dims) == dims
if (!squarable_length) {
cli::cli_abort(
message = c(
"Provided vector is not of a squarable length",
"{.arg {arg}} is of length {.num {x_len}}",
"This cannot be represented as a square"
),
call = call
)
}
}
check_if_not_numeric <- function(x,
arg = rlang::caller_arg(x),
call = rlang::caller_env()) {
if (!is.numeric(x)) {
cli::cli_abort(
message = c(
"Provided vector, {.arg {arg}}, must be {.cls numeric}, not {.cls {class(x)}}",
"We see that {.run is.numeric({.arg {arg}})} returns {.cls {class(x)}}"
),
call = call
)
}
}(in rewriting-ing this, I realised I don’t need to include the square root of the length to explain to the user, so I deleted that part of the message)
vector_to_square(data = 1:4)
#> [,1] [,2]
#> [1,] 1 2
#> [2,] 3 4
vector_to_square(data = 1:5)
#> Error in `vector_to_square()`:
#> ! Provided vector is not of a squarable length
#> `data` is of length 5
#> This cannot be represented as a square
vector_to_square(data = LETTERS[1:4])
#> Error in `vector_to_square()`:
#> ! Provided vector, `data`, must be <numeric>, not <character>
#> We see that `` is.numeric(`data`) `` returns <character>
This now uses data instead of x, which is what we want!
On writing error messages
I do think that writing good error messages is hard. Something that helped me think about this differently was something from the tidyverse style guide on error messages:
An error message should start with a general statement of the problem then give a concise description of what went wrong. Consistent use of punctuation and formatting makes errors easier to parse
They also recommend using cli::cli_abort(), which we used above. I’ve really enjoyed using this over stop, because, well, again, the tidyverse team summarises the reasons well, cli::cli_abort() is good because it:
Makes it easy to generate bulleted lists.
Uses glue style interpolation to insert data into the error.
Supports a wide range of inline markup.
Provides convenient tools to chain errors together.
Can control the name of the function shown in the error.
You should read the whole section on error messages, they’ve got great advice on how to write good error messages.
On input checking functions
This type of error message is often called an “input checking function”. There’s a really nice blog post “Checking the inputs of your R functions”, by Hugo Gruson, Sam Abbott, and Carl Pearson, which goes into more detail on the topic. They also recommend a few packages/functions for input checking that are worth checking out, such as checkmate, vec_assert(), vetr, and assertr to list a few.
Some more thoughts
In writing this blog post I’ve now realised that my pattern of creating check_ functions for input checking now also results in a potentially undesirable error for the user where they won’t know where the error has come from. I’ll need to make a note to change this in a lot of my packages.
In the future I’d really like to dive a bit deeper into adding classes or subclasses of errors. Mike Mahoney’s blog post, “Classed conditions from rlang functions” provides a good simple usecase of classed errors. And there’s a tidyverse design page on error constructors that I should read more deeply. I’m pretty sure this would be helpful in things like greta.
Functions are good
I will wrap up by emphasising a point about using functions. Good functions can be individually reasoned with, and used repeatedly across your work. This means you can write them once, use them many times, and only need to make changes to one place, rather than in many. Writing functions helps abstract away details, and helps clarify your code. They are an important building block for writing good code that has content that can be easily reasoned with, and extended.
Thanks
Thank you very much to the maintainers of cli (Gábor Csárdi), and rlang (Lionel Henry), and the team behind these packages. These are hard problems! Thanks to Adam Gruer for asking a great question that made me dive deeper into error checking. Special thanks to Maëlle Salmon for providing very useful comments that improved this post greatly, and thank you in general being a kind source of knowledge.
Is there anything I missed? Other questions / problems to explore in this space? Leave me a comment below if you want to chat about any of this :).
-
Thank you to Maëlle Salmon for helping me articulate this point, with inspiration from The Pragmatic Programmer). ↩︎