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

Getter and Setter are not called anymore once they crash. #520

Open
ltuijnder opened this issue Jan 7, 2025 · 2 comments
Open

Getter and Setter are not called anymore once they crash. #520

ltuijnder opened this issue Jan 7, 2025 · 2 comments

Comments

@ltuijnder
Copy link

ltuijnder commented Jan 7, 2025

I would like to implement an "initialization"-pattern with a S7 class. That is certain properties can only be accessed once the object is flagged as initialized (eg. all necessary resources have been allocated).

In the getter of the property I thus check if the object is initialized and raise an error if this is not the case. However, the getter seems to only run once and then afterwards the check is simply by-passed. I expected the getter to be called everytime.

The property in question also needs to be settable as it represent a resource that got allocated on initialization and thus needs to be assignable.

The following code highlights my problem:

library(S7)
Test <- new_class(
  "Test",
  properties = list(
    init = class_logical,
    conn = new_property(
      class = class_numeric, # In reality a connection object.
      setter = \(self, value) {
        self@conn <- value 
        self
      },
      getter = \(self) {
        if (!self@init) stop("Uninitialized")
        return(self@conn)
      }
    )
  )
)

t <- Test(init = FALSE)
t
# <Test>
# Error in (function (self)  : Uninitialized
t
# <Test>
#  @ init: logi FALSE
#  @ conn: int(0)
t@conn <- 2
t@conn
# [1] 2
t@init
# [1] FALSE

I know that the end-user could also perform a check if(!t@init). But then the check in the getter becomes pointless in the first place. I would like the class to self contained and guard against wrong usage of uninitialized properties.

@ltuijnder
Copy link
Author

Ok playing around a bit more. The root cause seems that once an error is thrown in the getter. The getter is not called anymore and the underlying value is returned. Even if the object went back to a valid state.

This seems not like desired behaviour because even if the end-user handled the error gracefully they are still left with an broken object where the getter does not work anymore.

Consider the toy example below where the value stored internally is 1 higher.

library(S7)
Test <- new_class(
  "Test",
  properties = list(
    a = new_property(
      class = class_numeric,
      setter = \(self, value) {
        print("setting")
        self@a <- value + 1
        self
      },
      getter = \(self) {
        print("getting")
        if (self@a == 10) stop("self@a = 10")
        return(self@a - 1)
      }
    )
  )
)
t <- Test(a = 1)
# [1] "setting"
t@a
# [1] "getting"
# [1] 1
t@a
# [1] "getting"
# [1] 1
t@a <- 9 
# [1] "setting"
t@a
# [1] "getting"
# Error in (function (self)  : self@a = 10
t@a
# [1] 10
t@a <- 1
# [1] "setting"
t@a # Getting is not called anymore. 
# [1] 2

@ltuijnder ltuijnder changed the title Getter is only called once Getter is not called anymore once it crashes. Jan 7, 2025
@t-kalinowski
Copy link
Member

Thank you very much for the bug report.

This is happening because the attribute .getting_prop is not cleared if an error is thrown from a custom getter.

I think we'll need to install a calling handler here, to allow for clearing the .setting_prop attribute. We'll likely need to do something similar for the setter.

That should also help with giving more informative tracebacks on errors from custom getters.

@ltuijnder ltuijnder changed the title Getter is not called anymore once it crashes. Getter and Setter is not called anymore once it crashes. Jan 21, 2025
@ltuijnder ltuijnder changed the title Getter and Setter is not called anymore once it crashes. Getter and Setter are not called anymore once they crash. Jan 21, 2025
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

No branches or pull requests

2 participants