Include settable dynamic properties in constructor

[t-kalinowski]

If a property has a custom setter(), it should be possible to set the property by passing a value to the default constructor. A draft PR, #445, implements this.

However, before #445 can be merged, we need to resolve the question: Should the default constructor call the property setter? There are three possible answers: Always, Never, and Sometimes. Each answer is motivated by a compelling use case.

(Discussion on this question has been organic and spread across different threads, this is my attempt to collate and help us reach a decision)

Never call setters

A compelling use case here is a set-once, read-only thereafter property.

Person <- new_class("Person", properties = list(
  birth_name = new_property(
    class = class_character,
    setter = function(self, value) stop("Cannot set 'birth_name' again")
  )
))

person <- Person(birth_name = "John")

try(person@birth_name <- "Bob")  # Error: Cannot set 'birth_name' again

In this scenario, new_object() only sets the underlying property attributes using attributes<-, never the property setter. There would then need to be a mechanism for class authors to opt-in to running the setters. This could be via a new_class(initializer=) hook:

Person <- new_class("Person",
  initializer = function(self) {
    props(self) <- props(self) # opt-in to calling all prop `setter`s
    self
  })

Sometimes call setters

The compelling usage example here is of a deprecated property. If explicitly set by the user, we want the setter to run; otherwise, we don't.

Person <- new_class("Person", properties = list(
  first_name = new_property(class = class_character), 
  firstName = new_property(                          
    getter = function(self) {
      warning("@firstName is deprecated; use @first_name instead")
      self@first_name
    },
    setter = function(self, value) {
      warning("@firstName is deprecated; use @first_name instead")
      self@first_name <- value
      self
    }
  )
))

hadley <- Person(firstName = "Hadley")   # warning
hadley@firstName                         # warning
hadley@firstName <- "John"               # warning

hadley <- Person(first_name = "Hadley")  # no warning

With this path, new_object() would need to inspect the input value and only conditionally invoke the setter via @<-.

new_object <- function(...) {
  props <- list(...)
  for (name in names(props)) {
    val <- props[[name]]
    if (!is.null(val)) # or missing(), or ...
      prop(object, name) <- val
  }
  validate(object)
  object
}

If is.null() is too strong a check for invoking the setter, we could instead use missing() (and also, make the corresponding constructor formal value quote(expr=)).

For this "deprecated property" use case, we may also want to demote the property from being a named argument in the constructor and instead accept it in .... This would also implicitly make it "missing" if not provided, and never set.

Always call the setters

The compelling use case here is of a property setter that does some more involved initialization, argument coercion, etc.

Person <- new_class("Person", properties = list(
  birth_date = new_property(
    class = class_Date,
    setter = function(self, value) {
      self@birthdate <- as.Date(value) # coerce when setting
    }
  )
))

person <- Person(birthdate = "1999-01-01") # coerces character to Date

Property authors could "opt-out" of calling the setter by including the appropriate checks within setter.

For example, going back to the "set-once" example, the setter would now need to handle initialization too:

Person <- new_class("Person", properties = list(
  birth_name = new_property(
    class = class_character,
    setter = function(self, value) {
      if (is.null(self@birth_name)) { 
        # initializing
        self@birth_name <- value
        return(self)
      } 
      # not initializing 
      stop("Cannot set 'birth_name' again")
    }
  )
))

The same would apply to the "Deprecated Property" scenario, but with different constraints.
Care would need to be taken not to accidentally invoke the getter() from the setter(). This could be done either by checking with attr(self, "firstName", TRUE) instead of self@firstName, or by finding another approach, such as accepting a "don't warn" sentinel like NULL in the setter:

Person <- new_class("Person", properties = list(
  first_name = new_property(class = class_character), 
  firstName = new_property(                          
    getter = function(self) {
      warning("@firstName is deprecated; use @first_name instead")
      self@first_name
    },
    setter = function(self, value) {
      if(!is.null(value)
        warning("@firstName is deprecated; use @first_name instead")
      self@first_name <- value
      self
    }
  )
))

[lawremi]

Under the category of sometimes, we could implicitly initialize the object with its default property values (what the methods package calls the prototype), and then only set the property (invoke the setter) if the passed value is different from the default. That simplifies the set once property, because the setter can always throw an error. It also avoids the warning when constructing an object with a deprecated property, even if there is a default in the constructor formals. For the transformation use case, as long as the default value is valid as an attribute, no transformation is needed, so it is OK if the setter is not called.

One question is how that would work with dynamic default values. Those could always be different, for example if the value is random or derived from the system time. Maybe those would need to be omitted from the prototype, and the birth date use case would have to check for whether the object is being initialized.

[t-kalinowski]

I recently updated the "classes and objects" vignette (https://rconsortium.github.io/S7/articles/classes-objects.html), and was reminded that the best approach is to choose the one that is easiest to teach and for users to discover. We should opt for the simplest approach that will lead to the fewest surprises. It's easy to imagine a frustrating user experience where, late in development, one discovers some additional hidden logic determining whether a setter runs or not. I'm hesitant to introduce any conditional logic that inspects the value of the setter argument to decide if the setter should run; users can just include that logic directly in the setter if they need to.

A "missing" argument is interesting, since it's not (in my mind) a value that a user would generate, so it's the equivalent of a "class-author" sentinel. However, it also makes it more difficult to build atop new_class() objects, where you may have to carefully check with missing() or pass along unforced promises to new_object(). I don't want to encourage routine use of a missing argument. The default "no-action" sentinel, I think, should be NULL or maybe a length-0 atomic, both of which can be handled at the R level without any special care.

I'm going to proceed with merging PR #445 as-is, given that it has two approvals, no objections, and currently implements the 'always' setter calling action. We can revisit this later and potentially relax the rule—perhaps by merging #446, which implements the 'skip-missing' feature, or by implementing 'skip-default,' 'skip-NULL,' or 'skip-!length' in another branch. However, I'd prefer to wait until we have a clearer understanding of the genuine need before introducing additional complexity.

[lawremi]

Since there is probably no better place to put this:

I remembered today that S4 was directly inspired by the object system in DYLAN (and indirectly CLOS). I found this documentation and was amused to find that they had to solve many of the same issues. They even have initializing a property (slot) with the current time as an example. Probably worth a read given how hard we have been thinking about this :)