Good practice to avoid mutating method parameters in Ruby?

Question

I understand that it's good practice to avoid mutating method parameters in Ruby (unless that's specifically the intention).

What is a simple coding rule of thumb (in terms of style, naming conventions, etc.) for how I should manage this?

For instance, is this good practice?

def array_of_three(a, b, c)
   d = a.dup
   e = b.dup
   f = c.dup
   # Do work on d, e and f
   return [d, e, f]
end

How should I name method parameters vs. the duplicates inside the function which can be mutated? (Assuming something like the above approach is "correct"!)

Thanks.

Answer 1

This is more of an extended comment on the other answers than an answer in and of itself, but if you're going to be mutating data you don't own, you should have the name of the method end in !. There's no fancy syntax for that; ! is just a valid character to end a method in. For example, look at this code:

a = [1, 2, 2, 3, 3, 3]
puts a.uniq
puts a

This will output [1, 2, 3], then [1, 2, 2, 3, 3, 3], as you'd expect. On the other hand, this code:

a = [1, 2, 2, 3, 3, 3]
puts a.uniq!
puts a

will output [1, 2, 3] twice, because it modified a in-place. This is especially nice if two things are true:

1. You could equally validly have a non-mutating and mutating version of the function
   • e.g. pop isn't a good fit for this pattern, because how do you pop without mutating?
2. There's some reasonable need for both.
   • This is a little fuzzier, but it's generally true on things that represent an operation on some data -- you probably want it to be able to both operate on data without changing it, and change it in place, for efficiency or just avoiding self-assignment or temporary variables.

This isn't a hard-and-fast rule, but it is a nice style to adopt, because it makes it easier to see at a glance that the thing is being mutated.

Note that if a had already been all unique elements (e.g. a = [4, 5, 6]) then uniq! would return nil, as is the standard for modify-in-place functions that don't end up performing a modification. I'd suggest that, if you adopt the ! style, you adopt this as well, to avoid confusion when it's compared against people's mental models of how !-functions work.

Answer 2

I know it is an answer based on my opinion, but this is my preferred way of "trapping" arguments within ruby (without resorting to .dup or the likes) is to pass them to a function, which passes the arguments to a lambda which is evaluated in place:

def argument_trap(*args)
  lambda { args }.call
end

def array_of_three(a, b, c)
  a, b, c = argument_trap(a, b, c)
  a += 1
  b -= 1
  c *= 2
  [a, b, c]
end

def array_of_n(*args)
  args = argument_trap(*args)
  args.map { |arg| arg += arg }
end

a, b, c, d, e, f = 1, 2, 3, 4, 5, 6

three = array_of_three(a, b, c)
n = array_of_n(a, b, c, d, e, f)

p [a, b, c]
p three
puts
p [a, b, c, d, e, f]
p n

Gives

[1, 2, 3]
[2, 1, 6]

[1, 2, 3, 4, 5, 6]
[2, 4, 6, 8, 10, 12]

Online demo here

With this pattern it is clear on the first function body line that the argument names are now pointing to clones. But again, this is a matter of preference - i like to keep the original argument names instead of naming them dupA or something like this, since for me it is clear that within these functions i am working now on duplicates.

This also prevents functions of doing both: working with references to the original value and with duplicates, which in my eyes is an advantage since doing both can lead to a lot of code confusion. On the other hand, one could forget an argument or two, and is again in coding hell..

Answer 3

Ruby's "good practices" generally boil down to not mutating data you don't own. The problem is that ownership can be somewhat hazy, so if in doubt you'll want to presume you don't own the data and make a copy if you need to mutate it.

The way bad behaviour manifests is an example like this:

a = [ 1, 2, 3 ]

do_stuff(a) # Works
do_stuff(a) # Doesn't do anything for some reason

a
# => [ ]    # Hey! Who did that?

Where let's say do_stuff was being a jerk and did this:

def do_stuff(a)
  while (v = a.pop)
    puts a
  end
end

That's damaging the arguments you're given, which is bad. Instead you should either pursue a non-destructive approach:

def do_stuff(a)
  a.each do |v|
    puts v
  end
end

Or instead make a copy if it's necessary to manipulate that structure, for example:

def do_stuff(a)
  a = a.uniq.sort
  while (v = a.pop)
    puts v
  end
end

A good way to test that your methods are behaving properly is to feed them frozen data in tests:

a = [ 1, 2, 3 ].freeze

do_stuff(a) # Fails trying to manipulate frozen object

I tend to write unit tests with aggressively frozen data (e.g. deep_freeze) to ensure that the arguments cannot be modified. This shakes out ownership problems really quickly.

If your function has a method signature like this:

def do_stuff(*a, **b)
  # ...
end

Then you will own both a (varargs) and b (kwargs) as they're broken out specifically for your method. In all other cases you need to be careful. This is where programming by contract is important as the caller of your method must know which arguments are handed over and which are not. The documentation must make this clear.