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.

There was a problem

You must be a member of this account.

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
  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