Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

errors.on(:att) should not return a String

#1903

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.

Reported by Justin French · February 7th, 2009 @ 05:11 AM

State: hold
Milestone: 2.x
Assigned to: nobody
Importance: none

Activity

  1. Xavier Shay
    Xavier Shay

    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

    February 7th, 2009 @ 05:21 AM

  2. Justin French
    Justin French

    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?'

    February 7th, 2009 @ 06:10 AM

  3. DHH
    DHH
    • 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()?

    February 7th, 2009 @ 01:41 PM

  4. Justin French
    Justin French

    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.

    February 8th, 2009 @ 12:54 AM

  5. Jonas Schneider
    Jonas Schneider

    +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)

    February 9th, 2009 @ 09:14 PM