constructive icon indicating copy to clipboard operation
constructive copied to clipboard

Improve extension system

Open moodymudskipper opened this issue 7 months ago • 2 comments

I struggled to provided a way for users to contribute code called by our functions.

This is usually done with S3 for classes but here we have different constructors, not classes of objects.

So I've implemented something using a environment in the package where these are stored.

However I have realised we might not need this, we can trick the S3 system to make our .cstr_construct methods generic themselves and treat constructors as classes.

This makes the boiler plate simpler, and less weird, despite the class hack, for example for the "uneval" class (this works right away without changing the rest of the package):

#' @export
.cstr_construct.uneval <- function(x, ...) {
  opts <- .cstr_fetch_opts("uneval", ...)
  if (is_corrupted_uneval(x) || opts$constructor == "next") return(NextMethod())
  UseMethod(".cstr_construct.uneval", structure(NA, class = opts$constructor))
  # rather than: 
  # constructor <- constructors$uneval[[opts$constructor]]
  # constructor(x, ...)
}

#' @export
.cstr_construct.uneval.list <- function(x, ...) {
  # same code as for current `constructors$uneval$list`
  .cstr_construct.list(x, ...)
}

#' @export
.cstr_construct.uneval.aes <- function(x, ...) {
  # same code as for current `constructors$uneval$aes`
 # ...
}

We can also eliminate the need for .cstr_fetch_opts() by collecting the dots in construct() into an opts arg, and we'll replace above opts$constructorby opts$uneval$constructor.

Note: with the above our constructors receive the same args as the high level method, so we cannot pass to them directly the additional args passed in the opts_ option. If we do pass an opt object the method itself can fetch those though. And we are more flexible because now every constructor can have its own arguments even if undocumented in the opts_ function (if we remove the restriction on empty dots).

Since .cstr_construct.uneval is a method itself it has a a .Class object equal to "uneval" in its environment, if we could use it we can have every generic object have the same body but then we can't call it directly anymore so probably not a good idea. However we can have a helper new_constructive_method(constructor) not to repeat this code and simplify the boilerplate. And Let's have a helper for opts_ functions too. These can already be done as part of #258.

moodymudskipper avatar Nov 23 '23 11:11 moodymudskipper

Since there's a lot of code to update I'd like to find a way to keep the old and new way coexisting. I think we can build some opts and args args and have our new methods use those, let's try first with this simple uneval class as a test.

moodymudskipper avatar Mar 17 '24 14:03 moodymudskipper

the opts arg should really be a named list, named with the class, collecting the opts_*()

moodymudskipper avatar Mar 17 '24 14:03 moodymudskipper