-
-
Save kbaribeau/3452778 to your computer and use it in GitHub Desktop.
#Do you prefer this? | |
merchant["thing"] = important_stuff.map { |important_thing| important_thing["units"] }.inject(&:+) | |
#Or this? | |
important_stuff.each { |important_thing| merchant["thing"] += important_thing["units"] } | |
#Why? Is it completely objective? Or are ruby's idioms/social mores/conventions the deciding factor? |
👍 to @bbasata's comment. The first one is trying to be too clever... no need for the map, just a reduce (ie. always reduce rather than inject).
I think the first one is easier to think about than @bbasata's because the reduce is much simpler (mostly for being commutative). When a non-commutative reduce can be expressed as a map and a commutative reduce, I like it.
I vote for:
#1 sum
#2 a pure inject
#3 the top one
#4 the bottom one
I think the top one is easier to follow. You're setting something to the result of something else. In the bottom one you have to look into the bits of the loop to realize there's a mutation going on.
# easy to see we're doing x = y
hash = {}
hash[:a] = [1, 2, 3, 4].map { |x| x + 1 }.inject(&:+)
hash[:b] = "asdf123"
hash[:c] = [2, 9, 8].map { |x| x * 2 }.inject(&:+)
# have to scan around to see what's going on
hash = { :a => 0 }
[1, 2, 3, 4].each { |x| hash[:a] += (x + 1) }
hash[:b] = "asdf123"
[2, 9, 8].each { |x| hash[:c] += x * 2 }.inject(&:+)
Forgot a :c => 0 in there
# easy to see we're doing x = y
hash = {}
hash[:a] = [1, 2, 3, 4].map { |x| x + 1 }.inject(&:+)
hash[:b] = "asdf123"
hash[:c] = [2, 9, 8].map { |x| x * 2 }.inject(&:+)
# have to scan around to see what's going on
hash = { :a => 0, :c => 0 }
[1, 2, 3, 4].each { |x| hash[:a] += (x + 1) }
hash[:b] = "asdf123"
[2, 9, 8].each { |x| hash[:c] += x * 2 }.inject(&:+)
I still like the .sum solution, but @fredericksgary's point is what makes me like #1 a little more than just an inject.
Is there something that makes a pure inject clearer than solution #1? Objectively, it's one loop vs two. You could argue that one loop is executing fewer instructions, but I think two simple loops is clearer than one (slightly) more complex one.
I prefer the second simply because it is more readable. If you're using Rails / ActiveSupport, you may also consider: