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.

Problem using merge conditions with orderedhashes

#3590

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

Reported by pedm · December 17th, 2009 @ 04:43 AM

State: resolved
Milestone: none
Assigned to: nobody
Importance: Low

Activity

  1. pedm
    pedm

    Uhh oh, I didn't format that right. What I meant was:

    @@ ruby

    2}>

    December 17th, 2009 @ 04:44 AM

  2. pedm
    pedm

    Okay one more try:

    <O_r_d_e_r_e_d_H_a_s_h {9=>2}>

    December 17th, 2009 @ 04:46 AM

  3. Ryan Bigg
    Ryan Bigg

    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.

    December 17th, 2009 @ 05:29 AM

  4. pedm
    pedm

    Thanks, that's a much better way of saying it!

    December 17th, 2009 @ 05:36 AM

  5. İ. Emre Kutlu
    İ. Emre Kutlu

    so will it be implemented or what? Lack of block syntax breaks the deep_merge, too.

    January 11th, 2010 @ 09:26 AM

  6. İ. Emre Kutlu
    İ. Emre Kutlu

    January 11th, 2010 @ 10:00 AM

  7. dohmoose
    dohmoose
    • 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?

    June 12th, 2010 @ 10:29 AM

  8. İ. Emre Kutlu
    İ. Emre Kutlu

    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?

    June 12th, 2010 @ 07:54 PM

  9. dohmoose
    dohmoose
    • 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

    June 13th, 2010 @ 05:14 AM

  10. İ. Emre Kutlu
    İ. Emre Kutlu

    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.

    http://gist.github.com/447516

    June 21st, 2010 @ 10:29 PM

  11. José Valim
    José Valim
    • State changed from new to resolved

    This was fixed on master a few days ago!

    June 22nd, 2010 @ 04:51 PM

  12. José Valim
    José Valim

    This was fixed on master a few days ago!

    June 22nd, 2010 @ 04:51 PM