#6483 ActiveSupport::OrderedOptions calls to_sym redundantly - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#6483 ✓committed

ActiveSupport::OrderedOptions calls to_sym redundantly

Reported by Ben Orenstein | February 28th, 2011 @ 01:31 AM

There's no need to call to_sym on the matched method name. The [] method already does this.

remove_to_sym_call.diff 1003 Bytes

Comments and changes to this ticket

Jeff Kreeftmeijer February 28th, 2011 @ 02:18 PM
- State changed from “new” to “verified”
- Assigned user set to “Santiago Pastorino”
- Importance changed from “” to “Low”
Verified. Applies cleanly to master and test pass. test_looping in ordered_options_test (https://github.com/rails/rails/blob/master/activesupport/test/order...) tests if strings are converted to symbols.

Nice one, Ben! :)
Ben Orenstein February 28th, 2011 @ 03:16 PM
Thanks Jeff!

If applied, this will by my first non-documentation, non-test-code patch :)
Josh Kalderimis February 28th, 2011 @ 03:39 PM
Hey Ben,

Generally the patch is good but you have also removed the require "ordered_hash", is there a particular reason?

The rule of thumb is if you only require a subset of ActiveSupport into your project, like ordered_options, everything that is required for the requires should also be setup and required correctly as well, eg. ordered_hash.

If you can fix this and include a new updated patch it would be appreciated.

Thanks,

Josh
Ben Orenstein February 28th, 2011 @ 03:56 PM
Oops. That change snuck in there accidentally.

Thanks for the catch, Josh.

This new patch fixes that.
- remove_redundant_to_sym.diff 827 Bytes
Josh Kalderimis February 28th, 2011 @ 03:57 PM
Hey Ben,

Thanks for the patch, looks good to me. +1.

Santiago, can you please take a look and do your magic :)

Thanks again Ben!

Josh
Repository February 28th, 2011 @ 06:44 PM
- State changed from “verified” to “committed”
(from [726599df984daca45afe5b969c74b416fcd6c11a]) Remove redundant to_sym call.

[#6483 state:committed]

Signed-off-by: Santiago Pastorino santiago@wyeworks.com
https://github.com/rails/rails/commit/726599df984daca45afe5b969c74b...