'{}' being 'bad practice' could go for some justification

[MichaelChirico]

adv-r/S4.Rmd

Lines 345 to 352 in 14bdb11

	It is bad practice to use `{}` in the generic as it triggers a special case that is more expensive, and generally best avoided.
	```{r}
	# Don't do this!
	setGeneric("myGeneric", function(x) {
	standardGeneric("myGeneric")
	})
	```

Casually, it seems strange to expect a noteworthy performance hit from using {. And it's ambiguous when we expect that performance hit -- package build time, or run time?

I glanced at the setGeneric() code and didn't immediately spot any "edge case", it probably requires more careful reading to figure this out.

Moreover, there are multiple examples in ?setGeneric itself that use {:

https://github.com/r-devel/r-svn/blob/43449c0a9d00390293ce9c0d7df855edfb10c0e2/src/library/methods/man/setGeneric.Rd#L322-L340

A link to an external reference here would be good.

[MichaelChirico]

I believe @mtmorgan originally inspired that section, and maybe @llrs can comment as well.

[mtmorgan]

I remember 'back in the day' that using {} would always insert a .local <- function(x) { ... }; .local(x, ...), so there was an additional function call and more importantly additional calling & parent environments. I think there were consequences for non-standard evaluation, object reference counts, and copy-on-change. But this is very long ago and (a) I probably mis-remember and (b) the situation has likely changed substantially. @lawremi might have something to say.

[MichaelChirico]

Thanks! I poked at the source of setGeneric() a bit and didn't find anything like that (including looking at the source ~10 years ago), am I looking in the wrong place?

https://github.com/r-devel/r-svn/blob/43449c0a9d00390293ce9c0d7df855edfb10c0e2/src/library/methods/R/Methods.R#L24

[llrs]

I remember something similar to Martin: that the definition with { and without them was not the same.

I think at some point it caused some problems on a Bioconductor packages and the fix was to simply remove it and the methods worked well (so no performance hit but resolution or environment problems). I don't remember when that happened but probably not that far but closer to ~5 years or so, because I think it was discussed on Bioconductor slack.

But I agree with the issue, backing this advice with a rationale or example of the problems it can cause would be good.