[LRUG] Feedback on Hash#dig_and_collect

[Tom Stuart]

Hi Alfredo,

Welcome to LRUG!

You didn’t mention whether you wanted feedback on the general idea/design or the specific implementation, but I have some brief feedback on the latter in case it’s useful:

* You do `keys.dup` so that you can safely mutate the array of arguments, but you don’t really need to do any mutation since `#dig_and_collect` is recursive anyway. A clearer alternative is to `def dig_and_collect next_key, *keys` so you can address the first argument directly rather than having to `dup` or `shift` anything.

* You’re using `next_val.is_a? Hash` and `next_val.is_a? Array` to decide what to return at the end of the method. I think the intent would be more obvious if you used a `case` statement instead:

case next_val
when Hash
  next_val.dig_and_collect(*keys)
when Array
  # …
else
  []
end

* You’re using `Enumerable#each_with_object` and `Array#concat` to gather up the results of the recursive calls in the `Array` case. This is the situation that `Enumerable#flat_map` is intended for, so you could achieve the same thing with `next_val.flat_map { |v| v.dig_and_collect(*keys) }` and I think that would make it easier to understand what is being returned.

Hope that helps!

Cheers,
-Tom