r-lib / R6

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

What's the recommended way to unit test private functions? #41

Closed jankowtf closed 9 years ago

jankowtf commented 9 years ago

Related: https://github.com/wch/R6/issues/40

jankowtf commented 9 years ago

That's my current approach:

MyClass <- R6Class(
  classname = "MyClass",
  public = list(
    initialize = function(
    ) {
      frames <- sys.frames()
      private$.private_env <- frames[[length(frames) - 1]]$private_bind_env
    },
    getPrivate = function() private$.private_env
  ),
  private = list(
    .private_env = "environment",
    foo = function() {
      "I'm a private function"
    }
  )
)

test_that("private/foo", {
  inst <- MyClass$new()
  expect_identical(inst$getPrivate()$foo(),
    "I'm a private function"
  )
})

Wonder if there's a better/more "built-in" way to unit test private functions.

wch commented 9 years ago

You could simplify it like so:

MyClass <- R6Class(
  classname = "MyClass",
  public = list(
    getPrivate = function() private
  ),
  private = list(
    foo = function() "I'm a private function"
  )
)

library(testthat)
test_that("private/foo", {
  inst <- MyClass$new()
  expect_identical(inst$getPrivate()$foo(),
    "I'm a private function"
  )
})

But of course this still involves adding the getPrivate function to your class, which you probably don't want to do in general.

You can actually access the private environment by inspecting a public function. In this example, it uses environment(inst$pub)$private to get the private environment:

MyClass <- R6Class(
  classname = "MyClass",
  public = list(
    pub = function() "I'm a public function"
  ),
  private = list(
    foo = function() "I'm a private function"
  )
)

test_that("private/foo", {
  inst <- MyClass$new()
  # Get the private environment
  priv <- environment(inst$pub)$private
  expect_identical(priv$foo(),
    "I'm a private function"
  )
})

This is diagrammed on the second page of https://github.com/wch/R6/blob/master/doc_extra/R6.pdf. The diagrams use the same style as those in Hadley's Advanced R book.

jankowtf commented 9 years ago

Ah, very nice! Thanks for the pointer!

zkurtz commented 6 years ago

Related: https://stackoverflow.com/questions/50222481/r-r6-class-as-a-rigid-read-only-data-structure

This conversation seems to be mostly a 'no' to my SO question. I wonder how hard it would be to assign the '@' symbol (or something like that) to provide built-in access to all private components in R6 classes by default. inst@foo would be quite convenient.

wch commented 6 years ago

@zkurtz As of now you can access the private environment via obj$.__enclos_env__$private. For example:

A <- R6Class("A",
  private = list(x=1)
)
a <- A$new()

a$.__enclos_env__$private$x
#> [1] 1

When this issue was originally raised, I don't think that was possible.

gaborcsardi commented 6 years ago

I usually just write a small internal get_private() function for this, and use it in the tests.

zkurtz commented 6 years ago

These are fine options, but all are bit unwieldy. I guess it's not clear to me whether there is a good reason that a$x (or a@x, if $ must be reserved for public) can't or should not be defined as a very brief version of a$.__enclos_env__$private$x, as per @wch's example.

Alternatively, getting back to the intent of my original SO post, it could be handy to have an option to 'lock' public components, such that x$b = 5 would return an error if b is locked.

jankowtf commented 6 years ago

A shortcut and a locking feature as proposed by @zkurtz would indeed be nice!

wch commented 6 years ago

The reason that it works that way is because that's how the $ operator works in R when indexing into an environment. You could define your own $ S3 method that does whatever you want, but it will come with a significant performance penalty, which may or may not be acceptable for your use case. One of the primary motivations behind R6 is to be lightweight.

I don't know if it's possible to do the same with @.

If you want to have a "locked" variable (which can only be changed from inside the class), you'll have to use an active binding, like this:

A <- R6Class("A",
  active = list(
    x = function(value) {
      if (missing(value)) private$.x
      else stop("Can't assign value to x")
    }
  ),
  private = list(.x = 1)
)

a <- A$new()

a$x
#> [1] 1

a$x <- 2
#> Error in (function (value)  : Can't assign value to x

Using an active binding like this will have a performance cost.

If you want a truly locked value (which can't be changed from inside or outside the class), you can use lockBinding() in the initialize function:

A <- R6Class("A",
  public = list(
    initialize = function() {
      lockBinding("x", self)
    },
    x = 1
  )
)

a <- A$new()

a$x
#> [1] 1

a$x <- 2
#> Error in a$x <- 2 : cannot change value of locked binding for 'x'
zkurtz commented 6 years ago

@wch Awesome, I'll try it out. If I understand correctly, lockBinding is exactly what I've been looking for (assuming it does not have big performance issues).

wch commented 6 years ago

@zkurtz Great. I believe that lockBinding doesn't have any performance cost.