diff --git a/R/SubsetDefinitions.R b/R/SubsetDefinitions.R index f34af45..ac69445 100644 --- a/R/SubsetDefinitions.R +++ b/R/SubsetDefinitions.R @@ -75,7 +75,7 @@ CohortSubsetDefinition <- R6::R6Class( #' @param overwrite if a subset operator of the same ID is present, replace it with a new definition addSubsetOperator = function(subsetOperator) { checkmate::assertR6(subsetOperator, "SubsetOperator") - private$.subsetOperators <- c(private$.subsetOperators, subsetOperator) + private$.subsetOperators <- c(private$.subsetOperators, subsetOperator$clone(deep = TRUE)) self }, @@ -189,7 +189,8 @@ CohortSubsetDefinition <- R6::R6Class( #' @field subsetOperators list of subset operations subsetOperators = function(subsetOperators) { if (missing(subsetOperators)) { - return(private$.subsetOperators) + # We don't want to return references to the operators in case users modify them after this + return(lapply(private$.subsetOperators, function(x) { x$clone(deep = TRUE) })) } checkmate::assertList(subsetOperators, types = "SubsetOperator") diff --git a/tests/testthat/test-Subsets.R b/tests/testthat/test-Subsets.R index b41aee7..65a247d 100644 --- a/tests/testthat/test-Subsets.R +++ b/tests/testthat/test-Subsets.R @@ -44,7 +44,11 @@ test_that("Subset definition", { checkmate::expect_list(listDef) expect_equal(length(subsetDef$subsetOperators), length(listDef$subsetOperators)) checkmate::expect_character(subsetDef$toJSON()) - + # check reference isn't passed + operators <- subsetDef$subsetOperators + # Operators should not be modfiable after being added to the subset definition + operators[[1]]$cohortIds <- 22 + expect_equal(subsetDef$subsetOperators[[1]]$cohortIds, 11) # Check serialized version is identical to code defined version subsetDef2 <- CohortSubsetDefinition$new(subsetDef$toJSON())