wahani / modules

Modules in R
https://cran.r-project.org/package=modules
Other
81 stars 3 forks source link

Thought: disable partial name matching on module's exports? #32

Closed mmuurr closed 3 years ago

mmuurr commented 3 years ago

Currently, this works:

mod <- modules::module(
  aaa <- "foo"
)
mod$aaa  ## correctly returns "foo"
mod$aa  ## returns "foo", but my brain wants NULL
mod$a  ## returns "foo", but my brain wants NULL

Since the returned mod object is a list, this makes sense. But(!), it doesn't quite mesh with how name-matching in environments works:

mod <- new.env(parent = emptyenv())
mod$aaa <- "foo"
mod$aaa  ## correctly returns "foo"
mod$aa  ## NULL
mod$a  ## NULL

I'd argue (and in fact often do!) that the partial-name matching is no good, as I've pretty frequently found myself with sneaky bugs because something matched when it shouldn't have, "thanks" to partial-matching. Yihui Xie agrees with me and to help prevent these bugs has included a strict_list in his xfun utility package. Perhaps the exported list object in modules should have strict name matching, too?

This would be a possibly-breaking change (though I'd again argue that anyone actually using partial-matching on purpose in code should be punished, and anyone using it accidentally would probably benefit from seeing the error :-). But it could also be included as an optional argument in export, e.g.

export(foo, bar, baz, .strict = TRUE)

... where the default would be .strict = FALSE to retain backwards-compatibility (at least until a major version bump, if you subscribe to semantic versioning).

Thoughts?

wahani commented 3 years ago

I'd argue (and in fact often do!) that the partial-name matching is no good, as I've pretty frequently found myself with sneaky bugs because something matched when it shouldn't have, "thanks" to partial-matching.

Yes. I agree. But. One of the key features of a module is that it is represented as R list. I tried to add as little as possible to that very well understood data structure to reduce any mental and coding overhead. Overriding the methods inherited from the super class (list) will break that contract.

By the way: the implementation of strict access would be trivial, not sure it is worth it. I will think about it some more.

mmuurr commented 3 years ago

One of the key features of a module is that it is represented as R list. I tried to add as little as possible to that very well understood data structure to reduce any mental and coding overhead.

Fair enough :-)