Commit 77fa9c32 authored by Delaigue Olivier's avatar Delaigue Olivier
Browse files

refactor: check the QupstrUnit argument using match.arg in CreateInputsModel

Refs #110
parent 6991a504
Pipeline #22242 failed with stages
in 8 minutes and 21 seconds
......@@ -216,9 +216,7 @@ CreateInputsModel <- function(FUN_MOD,
if(any(LengthHydro > 1000)) {
warning("The unit of 'LengthHydro' has changed from m to km in v1.7 of airGR: values superior to 1000 km seem unrealistic")
if (!(QupstrUnit %in% c("mm", "m3", "m3/s", "l/s", "L/s"))) {
stop("'QupstrUnit' must be one of these values: 'mm', 'm3', 'm3/s', 'L/s' or 'l/s'")
QupstrUnit <- match.arg(arg = QupstrUnit, choices = c("mm", "m3", "m3/s", "l/s", "L/s"))
  • Is there a problem with the following situation ?

    > QupstrUnit <- NULL
    > match.arg(arg = QupstrUnit, choices = c("mm", "m3", "m3/s", "l/s", "L/s"))
    [1] "mm"

    It's maybe exaggerated but we should prevent that a NULL value make a choice be default that it is not known by the user. I suggest to adopt one of these solutions:

    • if(is.null(QupstrUnit)) stop("QupstrUnit should be one of \"", paste(choices, collapse = "\", \""), "\"")
    • if(is.null(QupstrUnit)) warning("'QupstrUnit' has been redefined to \"mm\"")
    Edited by Dorchies David
  • Using tolower(QupstrUnit) fixed it and it allows to be flexible in the respect of the case of characters.

    > match.arg(arg = tolower(NULL), choices = c("mm", "m3", "m3/s", "l/s"))
    Error in match.arg(arg = tolower(NULL), choices = c("mm", "m3", "m3/s",  : 
      'arg' doit être un de “mm”, “m3”, “m3/s”, “l/s”
    > match.arg(arg = NULL, choices = c("mm", "m3", "m3/s", "l/s"))
    [1] "mm"

    I wasn't shocked by the previous behavior, because that's how the match.arg() function usually works.

  • And it is a normal behavior in R to return the default value if the NULL value is used.

    Edited by Delaigue Olivier
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment