RConsortium / S7

S7: a new OO system for R
https://rconsortium.github.io/S7
Other
387 stars 33 forks source link

Vignette about subclassing lists? #327

Closed jonthegeek closed 9 months ago

jonthegeek commented 1 year ago

I think this code is the kind of thing it should be ok for me to do:

person <- S7::new_class(
  "person",
  properties = list(
    name = S7::class_character,
    age = S7::class_integer
  )
)

# Create a class that expects a list of `person` objects.
people <- S7::new_class(
  "people",
  parent = S7::class_list,
  validator = function(self) {
    bad_people <- !purrr::map_lgl(
      self,
      ~ S7::S7_inherits(.x, person)
    )
    if (any(bad_people)) {
      "All values must be person objects."
    }
  }
)

people(list(person("Jon", 48L), person("Libby", 52L)))
#> <people> List of 2
#>  $ : <person>
#>   ..@ name: chr "Jon"
#>   ..@ age : int 48
#>  $ : <person>
#>   ..@ name: chr "Libby"
#>   ..@ age : int 52
people(list(logical(), character()))
#> Error: <people> object is invalid:
#> - All values must be person objects.

Created on 2023-08-27 with reprex v2.0.2

Assuming so, would a vignette explaining how to do that sort of thing be desirable?

jonthegeek commented 1 year ago

(presumably using vapply in the vignette but it was easier to knock out the example with purrr)

hadley commented 1 year ago

I think that's fine for you to do, but I'm not sure it's a common enough technique to warrant inclusion in a vignette here? For example, I think you'd want to discuss why you'd do that instead of vectorising the Person class, and I think that level of design discussion is starting to get out of scope.

jonthegeek commented 1 year ago

Fair enough, particularly since translating it to me real case, I don't think I actually have a good answer other than "because it was that way in the javascript object I'm starting from." I'll be back if I find a case where it makes more sense.

lawremi commented 1 year ago

Just a note that we could support homogeneous lists in S7 itself, or in a separate utility package. The List class from S4Vectors has proven very useful.

jonthegeek commented 1 year ago

Just a note that we could support homogeneous lists in S7 itself, or in a separate utility package. The List class from S4Vectors has proven very useful.

Do you have a specific example? I feel like the type of thing where you want to look up one full object at a time by name is where this is useful (ie, a JavaScript-Map-style object). The one I'm working with is a set of named variables for a server, where each variable has a vector of allowed values and a description. I want to look up one of them at a time to use it's properties. After sleeping on it, I'm back to feeling like the names are different from the other properties, and a wrapper class makes sense.

I COULD make this by moving "name" into the properties of the object, but it feels more natural to subset object_set by name, and then get back the full object I'm looking for.

szymanskir commented 1 year ago

Fair enough, particularly since translating it to me real case, I don't think I actually have a good answer other than "because it was that way in the javascript object I'm starting from." I'll be back if I find a case where it makes more sense.

It could be useful if we have a family of classes that share a common interface (methods that can be used to interact with them), but different in terms of the set of properties they have.

A good example is having different classes for storing data and displaying them as html in Shiny:

library(S7)

# Classes
entity_info <- new_class(
  name = "entity_info",
  abstract = TRUE
)

person_info <- new_class(
  name = "person_info",
  parent = entity_info,
  properties = list(
    name = class_character,
    last_name = class_character
  )
)

dog_info <- new_class(
  name = "dog_info",
  parent = entity_info,
  properties = list(
    name = class_character,
    breed = class_character,
    favourite_toy = class_character
  )
)

# Methods
render <- new_generic("render", "x")

method(render, person_info) <- function(x) {
  "Custom HTML for person_info class"
}

method(render, dog_info) <- function(x) {
  "Custom HTML for dog_info class"
}

# class storing a list of entities
entities_repository <- new_class(
  name = "entities",
  properties = list(
    entities = class_list
  ),
  validator = function(self) {
    bad_entities <- !purrr::map_lgl(
      self@entities,
      ~S7_inherits(.x, entity_info)
    )
    if (any(bad_entities)) {
      "All values must be entity_info objects."
    }
  }  
)

method(render, entities_repository) <- function(x) {
  paste(
    purrr::map_chr(
      x@entities,
      render
    ),
    collapse = "\n"
  )
}

entities_repository(
  entities = list(
    person_info(
      name = "Ryszard",
      last_name = "Szymanski"
    ),
    dog_info(
      name = "Fibi",
      breed = "Corgi",
      favourite_toy = "tennis ball"
    )
  )
) |> render() |> cat()

Another example use case would be generating a document composed of reusable segments, where reach segment is a separate class e.g. a segment with a table, a segment with a plot.

Those are some example cases where I see such a pattern to be useful!

Do you see a different way of handling such cases? 🤔

hadley commented 1 year ago

@szymanskir I don't have a specific counter proposal, but if I was implementing something like this, I would focussed on shared behaviour (i.e. having specific generics implemented) rather than relying on all objects having a shared base class. Your approach feels more like how I'd attack the problem in a message passing OO system, rather than a generic function OO system.

hadley commented 9 months ago

I'm closing this issue because I don't think it's quite concrete enough to fix with documentation, but we'll certainly continue to think about design patterns and best practices, and how we can help folks use S7 most effectively.