r-lib / R6

Encapsulated object-oriented programming for R
https://R6.r-lib.org
Other
405 stars 56 forks source link

new() inherits argument list from initialize() #103

Closed krlmlr closed 4 years ago

krlmlr commented 7 years ago

Fixes #47: The original boilerplate for new() is available as new_dots(). If a class has an initialize() method, new() is created as a function with proper argument list, which forwards to a local copy of new_dots().

All tests pass without modification.

Compared to #63, this also works when the new() method is assigned to a variable, a use case I've encountered recently.

wch commented 7 years ago

This doesn't seem to work for subclasses that inherit their initialize method. Could you make it work for those as well?

A <- R6Class("A", public = list(initialize = function(a = "", ..., b) NULL))
B <- R6Class("B", inherit = A)
B$new( <Tab>
wch commented 7 years ago

It feels a little weird to me to inline the whole new_dots function into the new function. Could the new function call out to a separate new_dots function instead?

krlmlr commented 7 years ago

Lazy evaluation of the inherit argument makes inheriting constructors super-difficult. I'll have to create an active binding for new(). Please confirm that you're willing to review/support such a solution.

wch commented 7 years ago

Oh, it makes sense that the dynamic inheritance would pose a problem for inheriting constructors. I don't think that having an active binding is worthwhile here.

krlmlr commented 7 years ago

Thanks. Why is inherit evaluated lazily anyway? Is it because otherwise you can't inherit a class that is defined after your class (in collation order)?

wch commented 7 years ago

It's because of #12.

krlmlr commented 7 years ago

Now the argument list is available only if there's an explicit initializer, otherwise we still use ... . All tests still pass locally.

hadley commented 7 years ago

I think this fixes #104. #47 is a slightly different issue.

wch commented 4 years ago

Closing this in favor of #191, which is based on the work in this PR.