Hi all,
After we got bit by it a couple times recently, I was thinking of submitting a PR for ActiveRecord that would issue a warning if you submit a hash containing the same key as both a symbol and a string to any of the AR construction methods (create, new, update_attributes, etc…) . As far as I can tell, the behavior when this happens in undefined (and change between Rails 3 and 4 for example).
For example:
attr = { ‘a’ => 123, :a => 456 }
ARModel.create!(attr)
In Rails 3, you’ll get an object with the ‘a’ attributes set to 456, in Rails 4, you’ll get 123.
I think this kind of things is almost always unintentional and worth a warning.
I’ll be happy to code this up and submit it if people think it would be useful and accepted.
Please let me know your thoughts and feedback.
Thanks,
Andrew
I’d label this purely as undefined behavior and such cases shouldn’t occur in the first place.
Andrew,
I agree they shouldn’t happen in the first place, but unfortunately they crop up in the real world, you get a hash from a JSON object and mix that with some params, and you might do this without even realizing it.
This is why I propose that AR spit out a warning message (perhaps upgraded to an error in future versions) if this happens.
Andrew
It’s going to be really expensive to check that on every call. Maybe it could be a good feature for a gem you only use in development.
Richard,
Thanks for your input. Perhaps controlled by a config variable (a la the old whiny_nils)? Can be off by default and turned on if desired what ever environments you want (probably dev and test)
Thanks,
Andrew
Perhaps controlled by a config variable (a la the old whiny_nils)?
I would prefer not. We are way overloaded with config options. Those options aren’t free, adding each feature behind a config flag requires maintenance and support. Instead have bundler be your config flag. If it’s important enough to add to Rails Core, it should be important enough to make its own gem. If that gem is so insanely wildly downloaded that everyone uses it then maybe we can consider adding it as a configuration option.