build_factory fails with NULL

[JenspederM]

Trying to use the build_factory() function. But encountered error when NULL's are present in the function.

factory::build_factory(fun = function(x) {
    c(x, NULL)
}, x)
#> Error in if (fn_body == target) {: argument is of length zero

I've tracked the error to occur in body_replace().

Perhaps it could be fixed by substituting if (fn_body == target) with if (!is.null(fn_body) && fn_body == target)

[jonthegeek]

I believe your solution will do it. Do you want to submit a PR? If not I'll try to knock it out Saturday. I can help you with the process if you haven't contributed to open source before.

[JenspederM]

I would, but I simply cannot figure out how to fix the bug (recursive functions are not my specialty). I have tried the following:

body_replace <- function (fn_body, target, replacement) 
{
  if (!is.null(fn_body) && fn_body == target) {
    return(replacement)
  }
  else if (length(fn_body) > 1) {
    for (i in seq_along(fn_body)) {
      fn_body[[i]] <- body_replace(fn_body[[i]], target, 
        replacement)
    }
  }
  return(fn_body)
}

And while the works for the simple function in the original issue. It fails other places, e.g., when I want to make a matrix factory without rownames, but with a new error:

build_factory(function(x) {
  matrix(x, ncol = 1, dimnames = list(NULL, "x"))
}, x)
#> Error in fn_body[[i]] : subscript out of bounds

Perhaps this is just a limitation of the package..

[jonthegeek]

I'll add these test cases to the test suite and figure out why they aren't working. It has to be doable, I just needed more cases!

I'll try to take a deeper look at this in a couple days. Thanks for the report!

[jonthegeek]

I'm fixing this in an upcoming PR, but just a warning that it probably won't work QUITE how you expect. The "x" in the dimnames will also be changed by the value of the "x" variable. I definitely recommend making the variable more explicit in the factory to avoid weird overlaps like that. Once the PR is merged, this will get you what you expect:

testing <- build_factory(function(x_vector) {
  matrix(x_vector, ncol = 1, dimnames = list(NULL, "x"))
}, x_vector)
x <- 1:10
testing(x)()
#       x
# [1,]  1
# [2,]  2
# [3,]  3
# [4,]  4
# [5,]  5
# [6,]  6
# [7,]  7
# [8,]  8
# [9,]  9
# [10,] 10

[JenspederM]

You’re awesome! Thank you 🙂

And the provided example is just that, in my use the names more meaningful. But thanks for the heads up. Perhaps it should be added to the DOC though?