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

Proposal: Introduce build_lazy/2 #402

Closed
germsvel opened this issue Nov 11, 2020 · 3 comments · Fixed by #408
Closed

Proposal: Introduce build_lazy/2 #402

germsvel opened this issue Nov 11, 2020 · 3 comments · Fixed by #408

Comments

@germsvel
Copy link
Collaborator

The problem

Many people build factories like this and run into uniqueness issues or foreign key issues:

insert_pair(:account, email: build(:email))

They think that each account will have a different email (especially if they set a sequence). The problem is that this is just function calling. So the above is equivalent to this:

email = build(:email)
insert_pair(:account, email: email)

In other words, people are trying to insert multiple accounts with the same email. They'll get a validation error if they have uniqueness.

Proposed Solution

The solution is to somehow delay evaluation of the email factory. We can do this in one of two ways:

Anonymous function

The simplest way to do this is to capture it in an anonymous function, basically doing this:

build_pair(:account, email: fn -> build(:email) end)

We could wrap that in a new function called build_lazy/2 (or possibly a different name), so that build_lazy/2 returns fn -> build(:factory_name) end

The trick then lies in the build/2 function. It now checks all attributes passed to it. If it's a function, it evaluates it. That way, when we're building a list of things, we evaluate the build(:email) once per account we're creating.

I built a proof-of-concept of this approach in c9816d2.

Private data structure

A more advanced implementation might create a struct that we can pattern match against when executing later.

So build_lazy/2 would return a data structure with all the information needed to execute the function later. The build/2 function would then perform a similar role as it does in the anonymous function case: it would pattern match on the custom struct and evaluate the "lazy" factory at the time when we're iterating through the attributes.

This is a proof-of-concept changing the anonymous function implementation to a struct-based implementation: afc0cb2.

Trade-offs

The anonymous function seems simpler to me, and I like that people could even use anonymous functions to delay execution instead of using build_lazy/2. But I don't know if that would be a breaking change. If people have been using anonymous functions in their factories as an attribute like this:

def user_factory do
  %{
    name: "Some Name",
    creation_callback: fn -> User.user_created!() end}
  }
end

We would all of the sudden evaluate those anonymous functions when building a factory because the anonymous function is now the mechanism by which we delay execution. It's not something I have done, so maybe no one has used ExMachina like that, but I also don't know all the cases for how ExMachina is used in the wild.

So the benefit of the private data structure approach over the anonymous function is that we know for sure no one is using that non-existent data structure, so we can safely ship it without potentially causing breaking changes.

Questions

I'd like to introduce one of these solutions, since it seems to be a pretty common problem. I believe these issues are all related: #221, #385, #373.

If you can answer any or all of these questions, it'd be great.

  • Do you currently use anonymous functions in your factories?
  • Which approach (anonymous function vs new data structure) seems better to you and why?
  • What do you think of the name build_lazy/2? Do you have suggestions for a better name?
  • Any other comments or suggestions?
@sgerrand
Copy link

Thank you @germsvel for taking the time to write this proposal and summarising potential approaches. I've been using anonymous functions to date in order to address this specific issue for factories with dependency chains.

As for the proposed solutions, I think introducing a function like build_lazy would help a lot of users of this package. In particular, I think it would be valuable to new users as it draws attention to what is otherwise a usability issue which only becomes apparent through specific use cases. Building a remedy for this into the library's interface demonstrates understanding and care! I like the name of the function as well, as I think "lazy" in this context should resonate with functional programmers.

@germsvel
Copy link
Collaborator Author

@sgerrand thanks for your response.

When you mention

I've been using anonymous functions to date in order to address this specific issue for factories with dependency chains.

Does that mean that the approach of usingbuild_lazy/2 with anonymous functions as their implementation would cause a breaking change for you? Or is that an approach that you prefer over the private data structure?

@sgerrand
Copy link

I don't really have strong opinions about either approach. I can see arguments for either direction.

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