This project is archived and is in readonly mode.
errors.on(:att) should not return a String
Reported by Justin French | February 7th, 2009 @ 05:11 AM | in 2.x
I find it very surprising and awkward that ActiveRecord's errors.on(:attribute) can return nil, a String (when there's only one error) or an Array (more than one error).
It's more common elsewhere in ActiveRecord to return an array even when there's only item... find(:all) is a perfect example.
I've attached a patch which changes the behavior so that an array is returned when 1 or more errors exist on the attribute, which will mean developers no longer have to write special code to handle the String special case.
This change is mostly backwards compatible:
Developers should have already been expecting either nil, a String or an Array of errors, so this change just negates the need for the String handling in their code.
However, there's probably a lot of validation unit tests out there expecting Strings rather than Arrays.
Comments and changes to this ticket
-
Xavier Shay February 7th, 2009 @ 05:21 AM
yes please, I hate the previous behavior. I want it to return an empty array rather than nil also, but that would probably break too much stuff
-
Justin French February 7th, 2009 @ 06:10 AM
yeah, i'd love an empty array too, and i started on that too, but I see a lot of code checking 'errors.on(:foo)' rather than 'errors.on(:foo).blank?'
-
DHH February 7th, 2009 @ 01:41 PM
- State changed from “new” to “hold”
This is not going to be backwards compatible and I'm sorta on the fence about whether it's even a good idea. If you're primarily working with 1 error per field, it'd be a hassle to deal with the array. And if you are thinking there might be multiple, couldn't you just wrap the call in Array()?
-
Justin French February 8th, 2009 @ 12:54 AM
I agree this isn't just a quick patch we can squeeze into stable any time we like, but I just don't buy the idea that most people, most of the time will only have or expect one error on a field. Nor can I buy the idea that they'd expect a String instead of an array for a pluralized method name like
errors.And (aside from unit tests) it's not THAT much of a compatibility problem, is it? For those that were expecting a string Array#to_s returns the string, and to_s is called by ERB.
I've seen plenty of developers have a "WTF?" moment on this. They all expected an array of one object. Hey, they all expected an empty array for zero errors too, so let's fix that!
Major releases like 3.0 are an opportunity to reduce those WTF moments.
-
Jonas Schneider February 9th, 2009 @ 09:14 PM
+1 for me, for sure. stumbled across this stuff so many times now... But it should be included first in Rails3 or something (where we break bc anyway :D)
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
Justin French