Skip to content

Commit

Permalink
Minor refactoring of validates_associated to replace #inject with #co…
Browse files Browse the repository at this point in the history 
…llect + #all?

[#1686 state:committed]
  • Loading branch information
@joshsusser @NZKoz
joshsusser authored and NZKoz committed Jan 22, 2009
1 parent 73cc5f2 commit ccda960
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/validations.rb
View file
Expand Up @@ -904,7 +904,7 @@ def validates_associated(*attr_names)
configuration.update(attr_names.extract_options!)

validates_each(attr_names, configuration) do |record, attr_name, value|
unless (value.is_a?(Array) ? value : [value]).inject(true) { |v, r| (r.nil? || r.valid?) && v }
unless (value.is_a?(Array) ? value : [value]).collect { |r| r.nil? || r.valid? }.all?
record.errors.add(attr_name, :invalid, :default => configuration[:message], :value => value)
end
end
Expand Down

8 comments on commit ccda960

@alloy
Copy link
Contributor

@alloy alloy commented on ccda960 Jan 22, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. all? takes a block as well:
unless (value.is_a?(Array) ? value : [value]).all? { |r| r.nil? || r.valid? }

@alloy
Copy link
Contributor

@alloy alloy commented on ccda960 Jan 22, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, the comment is not as seriously meant as github formats it…

@bjeanes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub comments needs a preview button for sure…

@dasch
Copy link
Contributor

@dasch dasch commented on ccda960 Jan 22, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

unless [*value].all? { |r| r.nil? || r.valid? }

@pivotal-legacy
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The semantics of this method are very specific and won’t work with the all? block. The enumeration must call #valid? on every value to force generation of all errors, then the #all? will return whether the whole set if valid.

@joshsusser
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ha, didn’t realize i was logged in as pivotal when i typed that previous comment. but i stand by it!

@stephencelis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[*value] would be an improvement over (value.is_a?(Array) ? value : [value]), though, both in readability and performance.

@stephencelis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ummm…half my comment disappeared? I was trying to say:

[*value]
would be an improvement over 
(value.is_a?(Array) ? value : [value])

Please sign in to comment.