Skip to content

Commit ea5d9e1

Browse files
hadleysckott
authored andcommitted
Code tweaks to evolution
By and large, I think the description of what you should do is good, but the code (IMO) is overly complicated, so I've proposed some simpler alternatives.
1 parent 25a7377 commit ea5d9e1

File tree

1 file changed

+14
-17
lines changed

1 file changed

+14
-17
lines changed

maintenance_evolution.Rmd

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,11 @@ Everyone's free to have their own opinion about how freely parameters/functions/
1414

1515
Sometimes parameter names must be changed for clarity, or some other reason.
1616

17-
A possible approach is to catch all parameters passed in to the function and check against some list of parameters, and stop or warn with a meaningful message.
17+
A possible approach is check if deprecated arguments are not missing, and stop a meaningful message.
1818

1919
```r
2020
foo_bar <- function(x, y) {
21-
calls <- names(sapply(match.call(), deparse))[-1]
22-
if(any("x" %in% calls)) {
21+
if (!missing(x)) {
2322
stop("use 'y' instead of 'x'")
2423
}
2524
y^2
@@ -29,12 +28,12 @@ foo_bar(x = 5)
2928
#> Error in foo_bar(x = 5) : use 'y' instead of 'x'
3029
```
3130

32-
Or instead of stopping with error, you could check for use of `x` parameter and set it to `y` internally.
31+
If you want to be more helpful, you could emit a warning but automatically take the necessary action:
3332

3433
```r
3534
foo_bar <- function(x, y) {
36-
calls <- names(sapply(match.call(), deparse))[-1]
37-
if(any("x" %in% calls)) {
35+
if (!missing(x)) {
36+
warning("use 'y' instead of 'x'")
3837
y <- x
3938
}
4039
y^2
@@ -44,7 +43,7 @@ foo_bar(x = 5)
4443
#> 25
4544
```
4645

47-
Be aware of the parameter `...`. If your function has `...`, and you have already removed a parameter (lets call it `z`), a user may have older code that uses `z`. When they pass in `z`, it's not a parameter in the function definition, and will likely be silently ignored -- not what you want. So, do make sure to always check for removed parameters moving forward since you can't force users to upgrade.
46+
Be aware of the parameter `...`. If your function has `...`, and you have already removed a parameter (lets call it `z`), a user may have older code that uses `z`. When they pass in `z`, it's not a parameter in the function definition, and will likely be silently ignored -- not what you want. Instead, leave the argument around, throwing an error if it used.
4847

4948
## Functions: changing function names
5049

@@ -72,21 +71,19 @@ bar <- foo
7271

7372
With the above solution, the user can use either `foo()` or `bar()` -- either will do the same thing, as they are the same function.
7473

75-
It's also useful to have a message but then you'll only want to throw that message when they use the old function name, e.g.,
74+
It's also useful to have a message but then you'll only want to throw that message when they use the old function, e.g.,
7675

7776
```r
7877
#' foo - add 1 to an input
7978
#' @export
8079
foo <- function(x) {
81-
if (as.character(match.call()[[1]]) == "foo") {
82-
warning("please use bar() instead of foo()", call. = FALSE)
83-
}
84-
x + 1
80+
warning("please use bar() instead of foo()", call. = FALSE)
81+
bar(x)
8582
}
8683

8784
#' @export
8885
#' @rdname foo
89-
bar <- foo
86+
bar <- function(x) x + 1
9087
```
9188

9289
After users have used the package version for a while (with both `foo` and `bar`), in the next version you can remove the old function name (`foo`), and only have `bar`.
@@ -103,11 +100,11 @@ To remove a function from a package (let's say your package name is `helloworld`
103100

104101
* Mark the function as deprecated in package version `x` (e.g., `v0.2.0`)
105102

106-
In the function itself, use `.Deprecated()` like:
103+
In the function itself, use `.Deprecated()` to point to the replacement function:
107104

108105
```r
109106
foo <- function() {
110-
.Deprecated(msg = "'foo' will be removed in the next version")
107+
.Deprecated("bar")
111108
}
112109
```
113110

@@ -139,7 +136,7 @@ In the function itself, use `.Defunct()` like:
139136

140137
```r
141138
foo <- function() {
142-
.Defunct(msg = "'foo' has been removed from this package")
139+
.Defunct("bar")
143140
}
144141
```
145142

@@ -149,7 +146,7 @@ In addition, it's good to add `...` to all defunct functions so that if users pa
149146

150147
```r
151148
foo <- function(...) {
152-
.Defunct(msg = "'foo' has been removed from this package")
149+
.Defunct("bar")
153150
}
154151
```
155152

0 commit comments

Comments
 (0)