ETL: minimal solution for approval

[emcoding]

There's a PR coming up for Mentor Notes for ETL This is the Minimal Solution for Approval that I currently have:

class ETL

  def self.transform(source)
    source.each_with_object({}) do |(score, letters), destination|
      letters.each { |letter| destination[letter.downcase] = score }
    end
  end
end

Or with inject and invert

def self.transform(source)
  source.invert.inject({}) do |destination, (letter, score)|
    letter.each { |letter| destination[letter.downcase!] = score }
  end
end

There's not a lot of recent examples. Are these equally approvable? Are there other approaches equally approvable? Other thoughts?

(ETL is meant to become a core exercise. Directly after Word Count, which is the first practice with each_with_object)

Edit (insti): ETL Readme

[emcoding]

This is the favourite I have seen so far:

        .flat_map { |value, tiles| tiles.product([value]) }
        .map { |tile, value| [tile.downcase, value] }
        .sort
        .to_h

[jackjennings]

The each_with_object version is what I’d anticipate seeing in Ruby, though the appeal of the more functional version is true. In the former, it seems strange to use two different kinds of block syntax. In the latter, is sort really required, given that hashes have no explicit order?

[emcoding]

Thanks @jackjennings !

• AFAIK sort is required only because of what the tests expect :-(
• I can change the block syntax to both using do..end, but to be honest, I don't have a problem with mixing them in this nested form. 🤔

[jackjennings]

I have seen some people make the argument that curly-braces blocks are for return values and do/end is for side effects, but for beginners I usually favor consistency over anything more clever.

Is it possible to fix the tests before it is promoted to not require an order? (Realizing that’s out of scope for this issue, it does seem like it impacts the recommended minimal solution).

Also, downcase! in the inject version can be the non-bang version (and probably should be) — downcase! returns nil if the string is already lowercase, which is a common gotcha.

[petertseng]

I think the sort can be omitted, with the following code as evidence:

things = 100.times.map { |i| [i, i].freeze }.freeze
puts 1000.times.count { things.shuffle == things }
puts 1000.times.count { things.shuffle.to_h == things.to_h }

output:

0 1000

I am making a wild guess that some variants that place a map inside the flat_map (to be more explicit than product) might see more use than product mostly because more students will think of it.

source.flat_map { |score, letters| letters.map { |letter| [letter.downcase, score] } }.to_h

[emcoding]

source.flat_map { |score, letters| letters.map { |letter| [letter.downcase, score] } }.to_h

So nice! Thanks.

And yes, the sort can be omitted. 👍

[SleeplessByte]

I have seen some people make the argument that curly-braces blocks are for return values and do/end is for side effects, but for beginners I usually favor consistency over anything more clever.

Rubocop is pretty clear I think :)

multi-line: do ... end single-line: { ... }

[jackjennings]

Yeah, I wouldn’t use do/end on a single line, but if I was writing this, I would no put the inside loop on a single line especially as it contains an assignment.

[tenebrousedge]

A variant on this has already been mentioned, but I prefer:

module ETL
  def self.transform(old)
    old.flat_map { |score,letters| letters.map(&:downcase).product([score]) }.to_h
  end
end

There's nothing wrong with nested blocks, but I find this a little easier to read. But, perhaps I value concision unduly.

[kytrinyx]

I'm going to go ahead and close this. If someone would like to move forward with anything in this discussion I would recommend opening a PR to add mentor notes in https://github.com/exercism/website-copy.