Skip to content

Instantly share code, notes, and snippets.

@kbaribeau
Created August 24, 2012 16:48
Show Gist options
  • Save kbaribeau/3452778 to your computer and use it in GitHub Desktop.
Save kbaribeau/3452778 to your computer and use it in GitHub Desktop.
Ruby style poll
#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?
@schoblaska
Copy link

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"]}

@mikelikesbikes
Copy link

👍 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).

@gfredericks
Copy link

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.

@richievos
Copy link

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(&:+)

@richievos
Copy link

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(&:+)

@kbaribeau
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment