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.

reverse_merge!() modifies the hash outside the method scope

#2884

Hi,

I have a Hash that is passed as a method argument to several methods on different Models. It has to be the same when it enters each method, but each Model can apply some specific default values to the Hash with reverse_merge!()

I'm surprised to see that in fact each method call modifies the original Hash, the one that is outside the scope of each method.

I don't doubt that it's an absolutly normal behavior from a Ruby perspective, but I don't think it's normal from the Rails perspective. At least I didn't expect this.

I've written a small rake task to illustrate this :

desc "bug reverse_merge!"
task(:reverse_merge) do
  
  options = {
    :attr1 => "generic string"
  }
  
  def some_method(options = {})
    default_options = {
      :attr1 => "default string",
      :attr2 => "another string"
    }
    options.reverse_merge!(default_options)

    puts "*** options INSIDE the method call :"
    puts options.inspect
  end
  
  puts "*** options BEFORE the method call :"
  puts options.inspect
  
  some_method(options)
  
  puts "*** options AFTER the method call :"
  puts options.inspect
  
end

Reported by Jérémy Lecour · July 8th, 2009 @ 01:51 PM

State: invalid
Milestone: 2.x
Assigned to: Ryan Bigg Ryan Bigg
Importance: none

Activity

  1. Mike Breen
    Mike Breen
    • Assigned user set to Ryan Bigg

    Ruby passes params by reference not by value so I'm not sure why we would expect Rails to behave differently.

    What you probably want to do here is to use the non-bang version.

    def some_method(options = {})
      default_options = {
        :attr1 => "default string",
        :attr2 => "another string"
      }
      my_options = options.reverse_merge(default_options)
    
      puts "*** options INSIDE the method call :"
      puts my_options.inspect
    end
    

    April 24th, 2010 @ 06:54 PM

  2. Ryan Bigg
    Ryan Bigg
    • State changed from new to invalid

    I agree with Mike here.

    April 25th, 2010 @ 12:14 AM

  3. Jérémy Lecour
    Jérémy Lecour

    I also agree with Mike. I guess I misinterpreted something. Sorry about that.

    April 25th, 2010 @ 10:11 PM