This project is archived and is in readonly mode.
Problem using merge conditions with orderedhashes
Reported by pedm | December 17th, 2009 @ 04:43 AM
Hi there - when I merge two ordered hashes, the merge works but does not respond to its more advanced settings. In particular, when I run the following code:
pos = Trade.sum("quantity", :conditions =>
{:buyer_id=>34}, :group => "traded_item_id")
neg = Trade.sum("quantity", :conditions => {:seller_id=>34},
:group => "traded_item_id")
pos.merge(neg){ |key, oldval, newval| oldval-newval }
The value that comes out is not the custom merge, but plain
old:
pos.merge(neg)
To put this in a bit more context,
pos = #2}>
neg = #4}>
and the result is:
4}>
when it should be:
-2}>
So yeah, I've been using ruby on rails for about a year, and
would love to help. So if someone could tell me where to start on
this, I'd love to solve this for you guys. Thanks!
Pat
Comments and changes to this ticket
-
pedm December 17th, 2009 @ 04:44 AM
Uhh oh, I didn't format that right. What I meant was:
@@ ruby
2}>
-
Ryan Bigg December 17th, 2009 @ 05:29 AM
I talked with pedm in #rubyonrails about this and we realised that merge for OrderedHash does not implement the block syntax of the merge from Hash.
-
İ. Emre Kutlu January 11th, 2010 @ 09:26 AM
so will it be implemented or what? Lack of block syntax breaks the deep_merge, too.
-
dohmoose June 12th, 2010 @ 10:29 AM
- Tag changed from merge, orderedhash to merge, orderedhash, patch
created patch for accepting block for merge.
İzzet Emre Kutlu, I put in a test for deep merge for ordered hash and it worked ok with no modifications, am I missing something? -
İ. Emre Kutlu June 12th, 2010 @ 07:54 PM
I check the tests. At test_deep_merge i think other_hash[:deep] not merged just returned.
I added a test to the gist http://gist.github.com/274118 .Can you please check this test? -
dohmoose June 13th, 2010 @ 05:14 AM
- Tag changed from merge, orderedhash, patch to merge, orderedhash
I replaced my test with your test, and also fixed the asserts (I had put in 'assert' not 'assert_equal'). Deep merge still worked, however, test_deep_block_merge failed. When I put in your deep merge code, that broke the 'test_deep_merge_on_indifferent_access' test in hash_ext_test.rb
This version of deep_merge passes all tests but its pretty ugly :(
What do you think?
http://gist.github.com/436339 -
İ. Emre Kutlu June 21st, 2010 @ 10:29 PM
I think "is_a?" method did the trick.
About being ugly, if i were you, i would write that code like this (which will be called uglier by most people :) ). So it is about the style.
-
José Valim June 22nd, 2010 @ 04:51 PM
- State changed from new to resolved
This was fixed on master a few days ago!
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>
People watching this ticket
Attachments
Tags
Referenced by
- 4838 [PATCH] Add Block Support to ActiveSupport OrderedHash merge This would resolve ticket #3590.