ActiveRecord::Result#to_hash doesn't return a hash

629 views
Skip to first unread message
Message has been deleted

Kache

unread,
Jan 22, 2016, 1:10:34 PM1/22/16
to Ruby on Rails: Core
Was about to open a GitHub issue, but found this mailing list


It's unexpected for `#to_hash` to _not_ return a hash, e.g. [AwesomePrint](https://github.com/michaeldv/awesome_print) checks and uses `#to_hash` to inspect hash-convertible objects, as noted in [an AwesomePrint issue](https://github.com/michaeldv/awesome_print/issues/215).

Ruby core even expects `Hash` from `#to_hash`:

    > Hash.try_convert(ActiveRecord::Base.connection.select_all("select * from users limit 1;"))
    TypeError: can't convert ActiveRecord::Result to Hash (ActiveRecord::Result#to_hash gives Array)

`#to_ary` already exists, meaning users of `ActiveRecord::Result#to_hash` should really be using `ActiveRecord::Result#to_a`

Kache

unread,
Jan 25, 2016, 1:16:52 PM1/25/16
to Ruby on Rails: Core
Deleted my double-posted message, also just to add for clarity: My suggestion is to deprecate/remove the ActiveRecord::Result#to_hash method, replacing all instances with #to_a or Array.wrap(result).

Kache

unread,
Jan 29, 2016, 2:40:02 PM1/29/16
to Ruby on Rails: Core
According to the linked guidelines for contributing @ Github/rails/rails, I should wait for positive feedback here before submitting an issue, but it also suggest that I "start writing code". Does this mean that I should open a pull request, or does this mean I should continue waiting for feedback?

Rafael Mendonça França

unread,
Jan 29, 2016, 2:44:36 PM1/29/16
to rubyonra...@googlegroups.com
That method seems to be private API of Rails (at least it should be) so go ahead to remove/deprecate it.

On Fri, Jan 22, 2016 at 1:10 PM Kache <kach...@gmail.com> wrote:
Was about to open GitHub issue, but found this mailing list.


It's unexpected for `#to_hash` to _not_ return a hash, e.g. [AwesomePrint](https://github.com/michaeldv/awesome_print) checks and uses `#to_hash` to inspect hash-convertible objects, as noted in [an AwesomePrint issue](https://github.com/michaeldv/awesome_print/issues/215).

Ruby core even expects `Hash` from `#to_hash`:

    > Hash.try_convert(ActiveRecord::Base.connection.select_all("select * from users limit 1;"))
    TypeError: can't convert ActiveRecord::Result to Hash (ActiveRecord::Result#to_hash gives Array)

`#to_ary` already exists with the same implementation, meaning that users of `ActiveRecord::Result#to_hash` should really be using `ActiveRecord::Result#to_a`

--
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-co...@googlegroups.com.
To post to this group, send email to rubyonra...@googlegroups.com.
Visit this group at https://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.

Kache

unread,
Jan 29, 2016, 5:48:12 PM1/29/16
to Ruby on Rails: Core
Ok, cool. I've never done this before, so I just wanted to make sure I'm doing this as expected by the community.

I've cloned the repo and gotten all the tests that already pass, passing. I repeated the same after my proposed change, and I'll submit a pull request once I've got everything organized.

Let me know if I should do anything differently.
Reply all
Reply to author
Forward
0 new messages