Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Abstract class validators are not inherited by child objects #329

Closed
jl5000 opened this issue Aug 29, 2023 · 2 comments · Fixed by #338
Closed

Abstract class validators are not inherited by child objects #329

jl5000 opened this issue Aug 29, 2023 · 2 comments · Fixed by #338

Comments

@jl5000
Copy link

jl5000 commented Aug 29, 2023

Even though you cannot create abstract classes, it's still very convenient to define validators within them to save having to define them for every child class.

class_test = S7::new_class("class_test",
                           properties = list(x = S7::class_double),
                           validator = function(self){
                             if(self@x >= 3) return("Error: should be less than 3")
                           })

class_test_child <- S7::new_class("class_test_child", parent = class_test)

class_test_child(1)
#> <class_test_child>
#>  @ x: num 1
class_test_child(4)
#> Error: <class_test> object is invalid:
#> - Error: should be less than 3

class_test = S7::new_class("class_test", abstract = TRUE,
                           properties = list(x = S7::class_double),
                           validator = function(self){
                             if(self@x >= 3) return("Error: should be less than 3")
                           })

class_test_child <- S7::new_class("class_test_child", parent = class_test)

class_test_child(1)
#> <class_test_child>
#>  @ x: num 1
class_test_child(4)
#> <class_test_child>
#>  @ x: num 4

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

@jl5000 jl5000 changed the title Abstract class validators do not run for child objects Abstract class validators are not inherited by child objects Aug 29, 2023
@mmaechler
Copy link
Collaborator

I agree. This has been a too narrow restriction for abstract classes.
Almost surely a patch would be welcome.

@hadley
Copy link
Member

hadley commented Sep 3, 2023

This looks like a fairly subtle bug — I think it's because constructors normally call validate() with recursive = FALSE because you expect the parent constructor to have done any validation it needs, but obviously we don't use the parent constructor when it is abstract. So I suspect we need a tweak to https://github.com/RConsortium/S7/blob/main/R/class.R#L261

hadley added a commit that referenced this issue Sep 8, 2023
hadley added a commit that referenced this issue Sep 8, 2023
hadley added a commit that referenced this issue Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants