-
-
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? |
I like that the first lends itself to immutability of merchant["thing"] and it reads as a value assignment.
Possible adjustment:
merchant["thing"] = important_stuff.inject(identity) { |things, important_thing| things += important_thing["units"] }
I prefer the second simply because it is more readable. If you're using Rails / ActiveSupport, you may also consider:
merchant["thing"] = important_stuff.sum{|important_thing| important_thing["units"]}
👍 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.
First line is trying to be too clever and isn't immediately obvious what it's trying to do (the
inject
at the end isn't helping it's clarity). It's also a tad longer and possibly involves more ops than the the 2nd line which immediately jumped out to me as "I'm building merchant['thing'] up from some important things!"