r-lib / R6

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

Fields containing reference objects - unwanted side effect #56

Closed jangorecki closed 9 years ago

jangorecki commented 9 years ago

Using introduction vignette example:

library(R6)
e <- data.frame(a=1,b=2) # my BIG DATA

SimpleClass <- R6Class(
    classname = "SimpleClass",
    public = list(x = NULL)
)
NonSharedField <- R6Class(
    classname = "NonSharedField",
    public = list(
        e = NULL,
        initialize = function() e <<- SimpleClass$new()
    )
)

n1 <- NonSharedField$new()
n1$e$x <- 1

n2 <- NonSharedField$new()
n2$e$x <- 2

# n2$e$x does not affect n1$e$x
n1$e$x
# [1] 1

e
#<SimpleClass>
#  Public:
#    x:

My variable e has gone.

It seems that I can safely rm(e) from my global env and both classes will still works. But polluting the global env, potentially wiping out important variable is an issue IMO. Could it be addressed somehow? For me this is not a big issue, but I'm developing package which uses R6 and non shared fields and user of my package may not be aware of the risk.

gluc commented 9 years ago

Why are you assigning e <<- SimpleClass$new() globally in the constructor? That overwrites your global e.

wch commented 9 years ago

@jangorecki If you want to use <<- assignment in R6 classes, you need to use portable=FALSE. Otherwise, with portable=TRUE (which is the default) you'd need to do self$e <- SimpleClass$new().

See this section of the intro vignette for more information: http://cran.r-project.org/web/packages/R6/vignettes/Introduction.html#using-self-and--

jangorecki commented 9 years ago

@gluc as this is a recommended way from vignette to use reference objects from R6 and not sharing them. @wch so the only way to use reference object and not sharing that object is making portable=FALSE, is that correct? It might be worth to mention that in vignette then.

wch commented 9 years ago

No, you can use portable=TRUE and self$e <- SimpleClass$new().

wch commented 9 years ago

I just realized one reason this might be confusing. R does something weird and surprising when n1$e is NULL, and you assign n1$e$x <- 1: it turns n1$e into a list.

library(R6)
NonSharedField <- R6Class(
    classname = "NonSharedField",
    public = list(
        e = NULL
    )
)

n1 <- NonSharedField$new()
n1$e
# NULL

n1$e$x <- 1
str(n1$e)
# List of 1
#  $ x: num 1

But the advice remains the same: make it portable and use self.

jangorecki commented 9 years ago

Thanks for your comments. I don't know why when I tried to use self$e <- SimpleClass$new() in initialize I was ending up with shared object of SimpleClass. I must made some stupid mistake because now when I tried the way you describe it works like a charm. Thanks again! Still the vignette for using non-shared objects within R6 could mention self$ <- way, not only <<-.

wch commented 9 years ago

@jangorecki Ah, I didn't realize that your code was taken directly from the vignette! Sorry about that. That's definitely a bug in the vignette, and I'll fix it soon.

jangorecki commented 9 years ago

I'm glad there is at least some constructive input from that ticket. Thanks for great pkg dev!