4

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.

5
  • I see there are two votes to close. That doesn't mean it's not a good question; just that it requires a subjective answer, something discouraged on SO. I don't think there's a naming convention for a.dup. Speaking for myself, I'm often dissatisfied with whatever I choose. I suggest you try to "Do work on a, b, c that does not mutate those values". If you must dup consider doing it "in-line" to avoid the creation of a variable. For example, a.dup.each {...}. Commented Jun 28, 2018 at 20:34
  • Thanks – sorry, to go off-topic a second – could you explain what you mean in terms of why this question is considered inappropriate? Not quite understood. Thanks. EDIT: No, I think I get you – but to clarify – I wanted to check if there was either (a) a "right" way to do this and more importantly, (b) ensure I avoided a wrong way/pitfall (specifically if my "for instance" above counted as one). Thanks! Commented Jun 28, 2018 at 20:37
  • Click on "What types of questions should I avoid asking?" under "Asking" at the Stackoverflow help center and read the points under, "To prevent your question from being flagged and possibly removed, avoid asking subjective questions where …". Commented Jun 28, 2018 at 20:45
  • Cheers – naturally I'll say I feel I'm in the constructive camp :) Commented Jun 28, 2018 at 20:48
  • 1
    There's no correct answer, but mutating parameters makes things much harder to test and debug when things go wrong, because spooky action at a distance. Commented Jun 28, 2018 at 21:41

3 Answers 3

6

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.

4
  • I think it'd be worth mentioning the ! convention -- i.e. that it marks in-place modification. For example, for a = [1, 2, 2, 3, 3, 3], a.uniq will return [1, 2, 3], and a.uniq! will return that and make a now also equal [1, 2, 3]. Similar examples abound -- map/map!, shuffle/shuffle!, etc.
    – anon
    Commented Jun 28, 2018 at 22:08
  • @NicHartley That's a good note, but it's also important to remember that's not universal. pop and shift modify in-place but aren't ! flagged. Sometimes ! just means "raises exceptions".
    – tadman
    Commented Jun 28, 2018 at 22:21
  • Good explanation and suggestions. Thanks. Commented Jun 29, 2018 at 16:13
  • 1
    @tadman As I wrote that comment I realized it needed more explanation, so I wrote my own answer. (That is why I said "you should mention" rather than attempting to explain the whole thing)
    – anon
    Commented Jun 29, 2018 at 16:21
2

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.

0
1

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

1
  • Thanks for the detailed explanation. I agree, I want to avoid confusing code with additional variable names and names like a_dup, etc. My solution for the time being has been to name arguments with underscores to indicate you should leave them alone, e.g. def a(_b, _c, _d) and then if I need to do anything destructive, take b = _b.dup. Commented Jun 29, 2018 at 16:11

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.