Add function input checks

[matt-dray]

Accept only 'correct' argument inputs for each function. Provide useful error messaging to users on failure.

This will impact the range of tests that can be developed as part of #7.

It's probably best to put these checks in a utils.R file, given that similar checks can be used across functions.

This applies to the core functions that exist at time of writing. More checks will be required as the package's functionality expands.

Perhaps start with base stop() to begin with. Could make later use of the {cli} package for a slicker interface.

[chrismainey]

Great idea Matt. are you putting this together? I'm going to start adding some tests as part of #7 .

[matt-dray]

Adding my Slack comment here for visibility and discussion. Perhaps this should start with simple check_type() and check_length() functions, or a single function that handles both of those things. One question might be whether we're strict about integer input for some of the values.

[chrismainey]

I think we probably have to handle integers, as it should probably be possible for people to supply decimal inputs (e.g. 25.5 week target). It's probably not common, but it shouldn't bug out. I like the idea of check_type() function and check_length() in utils. Versatile then I suppose.
Presume use would be, something like check_type(input1, "expected type") e.g. check_type(mean_wait, "double")

Also need a descriptive message for when inputs are missing.

[matt-dray]

Bingo, should be nice and versatile. It can be messy to make a lot of check_*() calls (relief_capacity() has four arguments that need checking for class and length, for example), so I've been considering a check_inputs() function that takes the supplied values and runs the check_*() functions over them all.

Phrasing is currently along the lines of:

The input to the 'target_wait' argument must be of length 1. You provided an input with length 5.

This phrasing can be changed, of course. I was thinking about the Tidyverse style guide section on error messages.

And yes, good point on missingness, thank you.

[matt-dray]

Incidentally, I don't actually know what the best practice is for input checks like this. This is just how I've done it in the past! Advice very welcome.

[chrismainey]

I think we are in the same boat here. I've written checks in the past, but they've been very bespoke and individual, so using a generic function is a step up for me. I've not heard anyone speak about that before either, I've just gleaned it from my own packages breaking with bad inputs and confusing users. For brevity, we could always call using something like lapply(list(input1, input2), check_*) rather than write it out multiple times.

[ThomUK]

Someone sent me this article today, which has elegant ways of making sure the error messages reference the functions that the user is using, rather than the content and arguments of the checking functions.

It's beyond anything I've implemented before, but I've made a mental note to consider it for my most-used packages. I usually stop with check functions and assumed that the slightly confusing error messgaes were a necessary consequence of cleaner reading code.

https://www.njtierney.com/post/2023/12/06/long-errors-smell/

[matt-dray]

Oh wow, Nick Tierney is clearly on our wavelength. As it happens, I was coding something earlier that looks basically like what he's come up with there. Except the check functions I've written take multiple inputs via .... That means we can collect and check multiple arguments at once, which is handy when (a) we have lots to check (e.g. one of the functions needs to have five arguments checked for their class) and (b) when there's a variable number of things that need comparing to each other (i.e. in the case of the check for matching argument lengths). I'll try and get this into a PR and will explain in tomorrow's meeting if I may.

[matt-dray]

Note to self/us: there was also an R-Hub post on {cli} tips and tricks recently.