Ambiguous documentation on `initialize_with` behaviour #622

Closed
lime opened this Issue Feb 27, 2014 · 5 comments

Comments

Projects
None yet
2 participants
Contributor

lime commented Feb 27, 2014

In the GETTING_STARTED section on custom construction it is noted that:

Notice that I ignored the name attribute. If you don't want attributes reassigned after your object has been instantiated, you'll want to ignore them.

while later on that statement seems to be retracted:

When using initialize_with, attributes accessed from within the initialize_with
block are assigned only in the constructor [...] This prevents duplicate assignment

Is the first quote outdated? If so, should the code examples be changed not to use ignore, in addition to changing the wording of the text?

Owner

joshuaclayton commented Mar 4, 2014

@lime I reference ignoring attributes because without doing so, the attribute will get set twice.

In the example

factory :user do
  ignore do
    name "Jane Doe"
  end

  email
  initialize_with { new(name) }
end

build(:user).name # Jane Doe

this would run

user = User.new('Jane Doe')
user.email = 'person1@example.com'

Without the ignore block, it'd run

user = User.new('Jane Doe')
user.name = 'Jane Doe'
user.email = 'person1@example.com'

Hopefully this provides some insight into why I discuss ignored attributes after initialize_with - if you have any suggestions for how it could be worded more clearly, please let me know!

Contributor

lime commented Mar 4, 2014

Thanks for the reply!

I think that part of the instructions is worded clearly enough, I interpreted it exactly as you described above. :)

It's the last part of the section (the two last code examples before "Custom Strategies") that seem to contradict the rest, claiming that factory_girl >= 4.0 prevents duplicate assignment automatically. Is that the case? I tried to find the relevant code, but I am not completely sure where to look.

Contributor

lime commented Mar 4, 2014

It looks like a5b3a97 is where this functionality is introduced. To me it seems like it indeed renders the ignore blocks obsolete in regards to initialize_with, or am I missing something?

Owner

joshuaclayton commented Mar 5, 2014

@lime now that you mention it - yes, this isn't necessary anymore. Would you be interested in making a pull request updating the doc?

Contributor

lime commented Mar 5, 2014

I'll give it a shot later in the week. :)

lime added a commit to lime/factory_girl that referenced this issue Mar 7, 2014

Clarify documentation on using initialize_with
Using ignore blocks to avoid duplicate assignment is obsolete since
factory_girl 4.0.

Closes #622
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment