Repository navigation
Is it good to use reverse_merge! on the optional hash argument of a method? #99
Description
I found an example in this guide
def obliterate(things, options = {})
default_options = {
:gently => true, # obliterate with soft-delete
:except => [], # skip obliterating these things
:at => Time.now, # don't obliterate them until later
}
options.reverse_merge!(default_options)
...
end
I'm wondering whether this is a good practice. When you use this method, the input options hash might be modified by reverse_merge! method, which, in my opinion, is very error-prone process in a big project.
Reacted by Nebojša Stričević
Activity
robotpistol commentedon Jun 13, 2016
I've been thinking about this for a while. and I think I agree with you. Though many options are passed in statically, it is very possible that they are constructed and reused elsewhere
In this case it is safer to do
Which will set
optionsto a copy of the original options hash, leaving the original in tact.ljharb commentedon Jun 13, 2016
+1, any methods that mutate imo should be avoided, to avoid this hazard.
Alternatively,