Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support "classical" factories with force() calls #38

Open
lhdjung opened this issue Nov 10, 2022 · 2 comments
Open

Support "classical" factories with force() calls #38

lhdjung opened this issue Nov 10, 2022 · 2 comments

Comments

@lhdjung
Copy link

lhdjung commented Nov 10, 2022

I'm a big fan of the package, and I hope build_factory() will become a new standard. However, some developers might not want to rewrite existing factories that way.

Such factories will likely need force() calls. My idea here is to include one or two new functions in the package that help to make sure no call is missing. They would take a function factory and print force() calls for their arguments, so that people can simply copy and paste them into the source code. Sorry if this is beside the point of the package – I'm curious what you think.

I wrote two dependency-free functions as described. They differ in whether they simply print force() calls for every argument or try to search the factory's body for the start of the output function and check which arguments are covered by force() calls before that point. (This is just a heuristic, of course.)

Here is an example where 3 force() calls are needed but only 1 is present:

> fun <- function(x, y, z) {
+     force(x)
+     function(alpha, beta) {
+         out <- x * y + z
+         (out - alpha) / beta
+     }
+ }
> 
> force_print(fun)
2 out of 3 arguments are not covered by `force()` calls.
Consider including these calls in `fun()`
before the start of the output function:

force(y)
force(z)
> 
> force_print_all(fun)
force(x)
force(y)
force(z)
@jonthegeek
Copy link
Owner

I'm glad to read that you like the package! I haven't updated it in quite a while now, I'll try to use this as a push to do so!

That's definitely an interesting idea! I can imagine having a third function that takes a traditional factory (with or without forces), and converts to the rlang equivalent!

I'll think about this some more, then try to sort out implementation!

@lhdjung
Copy link
Author

lhdjung commented Nov 11, 2022

That's great! I like the fact that users can simply copy and paste the output of build_factory() instead of relying on the package as a dependency. This feature is quite distinctive and might convince some developers to use factory in the first place. You mention it at the end of vignette("building_a_factory"), but maybe it could be advertised more upfront, so that people casually browsing the package learn about its advantages?

(For what it's worth, the force_print*() functions are also made for that kind of interactive use, but I really don't want to push them.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants