Checking arguments redundant?

[VeruGHub]

Is it necessary to check the arguments climatic_var in both functions get_daily_data and build_url considering than the second one is designed to be called in the first one?

  if (!climatic_var %in% c("Tmax", "Tmin", "Prcp"))
    stop("climatic_var must be one of 'Tmax', 'Tmin' or 'Prcp'")

[Pakillo]

I think it's good to keep that check in both places, for several reasons:

• it's not costly at all (from the computational point of view)
• If you keep the check only in build_url, you'll do a lot of unnecessary computations before the code breaks up at L160 when build_url is called.

So I'd definitely keep it in both places

[VeruGHub]

It is not costly at all, but it is also not necessary to check those arguments inside build_url since this function only can be called inside get_daily_climate, right?

[Pakillo]

Well, build_url could be called directly by any user, but it's true that it is designed to be called within get_daily_climate. In general I like to be more defensive than not and ensure that, whenever possible, all functions return sensible (and early) errors if the arguments provided are not correct. So I'd keep the check as it's not costly, just in case someone calls that function directly. But if you want to remove the check in build_url the package will keep working exactly the same, so I'm fine with that if you want to do so. Up to you!

[VeruGHub]

Alright. Let's keep it. It is true that build_url can be directly called so then it is better to be cautious. Thanks for your opinion!