Suggestion to use Hash#slice in the Raindrops mentoring notes

[s2k]

In one of the sessions I mentored a student came up using Hash#slice in her solution to Raindrops. (In the end she removed that call while making other changes, though.)

Over in the Slack track @F3PiX suggested (https://exercism-team.slack.com/archives/CARVB5V0R/p1560869977017500) to open this issue to discuss whether this solution should be incorporated in the mentoring notes.

A solution using Hash#slice could look like this:

class Raindrops
  SOUNDS = {
    3 => "Pling",
    5 => "Plang",
    7 => "Plong"
  }

  def self.convert(num)
    string = SOUNDS.slice(*sound_factors(num)).inject('') { |str, sound| str + sound[1] }

    string.empty? ? num.to_s : string
  end

  def self.sound_factors(num)
    SOUNDS.keys.select { |n| num % n == 0 }
  end
end

[s2k]

Come to think of it for a while now, I prefer the solution using Hash#select:

SOUNDS.select { |key, _| (num % key).zero? }

[kotp]

I would change sound_factors(number) to factors because that is the collection you would get back. And I prefer number to num always. English and Acronyms and all that... full words are easier to look up.

sound vs string since writing to the problem being solved, versus to writing to the programming domain.

And SOUNDS as a name only describes half of the Hash object, where RULES may describe the idea being held, while FACTOR_TO_SOUND may have it's benefits as well.

Also, there is some discussion that should probably come up with the public method being introduced... we need convert on the class or module, as enforced by the tests, and that is the only public interfaced prescribed.

[petertseng]

I suggest Hash#values_at rather than Hash#slice and then it is possible to use Array#join instead of Enumerable#inject.

[joshgoebel]

there is some discussion that should probably come up with the public method being introduced

Agreed. With a local variable instead:

class Raindrops
  FACTOR_TO_SOUND = {
    3 => 'Pling',
    5 => 'Plang',
    7 => 'Plong'
  }

  def self.convert(number)
    factors = FACTOR_TO_SOUND.keys.select { |n| (number % n).zero? }
    sounds = FACTOR_TO_SOUND.values_at(*factors).join

    sounds.empty? ? number.to_s : sounds
  end
end

Not sure this is any better than the last example already listed in the mentor notes as it does this same thing with only select:

SOUNDS.select{|key, _| (num % key).zero? }.values.join
# Though I see map/join more often:
SOUNDS.map{|key, sound| sound if (num % key).zero? }.join

Looking at them side by side the former looks nicer.

[kotp]

I am not a fan of needing to use the hash twice, in the full class write up example above. I think it focuses on the wrong thing as far as the solution goes. The focus is probably best when building a string more directly.

The same is true for the original post, where the Hash is relied on more than once.

[kytrinyx]

If there is more to be discussed here I would recommend opening a PR on the mentor notes for the exercise.

I'm going to go ahead and close this issue.