boomer icon indicating copy to clipboard operation
boomer copied to clipboard

corner cases with function factories

Open moodymudskipper opened this issue 3 years ago • 2 comments

rigged functions have the same code as the original function, however this is not the case of wrapper functions.

So if wrapper functions are returned, or if the rigged function does anything with their body, the result will be unexpected :

image

image

I'm not sure if it can be improved, it's also hard to document.

If the mask contained active bindings rather than wrapper functions, we might make the active binding check how it's used, and if it's not used as a calling function, return the original function, but I haven't figured out how to make the active binding know how it's used.

https://stackoverflow.com/questions/68090266/how-to-have-an-active-binding-know-if-its-called-as-a-function

moodymudskipper avatar Jun 22 '21 19:06 moodymudskipper

A way out of some of of those corner cases might be to wrap the last line in a return() if it's not there already, then shim return so it convert the function back to original.

We might also have special shims for body(), args(), formals() ... But I feel there'd be no end.

Maybe better wait for actual problematic cases to come up

moodymudskipper avatar Jun 22 '21 22:06 moodymudskipper

Some more thoughts.

With a package like this one, corner cases are inevitable, even {knitr} (so also {reprex}) don't always work if the code uses sys.calls() or tries to print some characters on some systems.

Thus every new fix will bring new problems.

This might not be a good reason not to fix things, but when we're talking about rare corner cases and that the fix looks like a hack, they might be better be optional.

For instance we might have a boom.return_unwrapped that shims return (and wraps a return call around the last call if it's not there) to make sure the returned object is an unwrapped function, but it would be FALSE by default not to add confusion to an already complicated package. It would be documented properly for advanced users who know what they're doing.

We might have an option ton enable the wrapping of functions defined at run time with <- through build_shimmed_assign, right now it always happens. This is what creates the issues above in the first place, but I'd rather keep it enabled by default because it's handy.

We need a good help page for options, which will refer to a good vignette about how boomer works.

moodymudskipper avatar Jun 24 '21 10:06 moodymudskipper