current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite
1

I want to collect my users' data as an array:

h = {}
a = [["user_3", 765], ["user_1", 2], ["user_1", 1], ["user_2", 124],["user_1", 3], ["user_2", 223], ["user_2", 334]]
# Expected output :: {"user_3"=>[765], "user_1"=>[2, 1, 3], "user_2"=>[124, 223, 334]} 
a.each do |x|
  if h.keys.include?(x.first)
    h[x.first] = (h[x.first] << x.last)
  else
    h[x.first] = [x.last]
  end
end

Everything is working fine in my Ruby script, but is there any better way to achieve my result?

share|improve this question
    
Relevant: github.com/rubyworks/facets/blob/master/lib/core/facets/…. –  tokland Jun 11 at 8:04

2 Answers 2

up vote 6 down vote accepted

You very rarely need to modify a variable from inside an each block.

Instead, check out all the methods available to you in Array and Enumerable.

Among them, there's #each_with_objects, which you can use like so:

a.each_with_object({}) do |(name, id), hash|
  hash[name] ||= []
  hash[name] << id
end

which'll produce the same hash as your current code.

You could also do:

a.group_by(&:first).map do |name, pairs|
  [name, pairs.map(&:last)]
end

That returns an array of arrays, but you can turn that into a Hash with Hash[]. You may not need to, though. A lot of things treat an Array object with [key, value]-style elements the same as a Hash object. Depends on your needs.

share|improve this answer
2  
For the first, you could also use Hash.new { |h, k| h[k] = [] } in place of {} and get rid of the hash[name] ||= []. –  QPaysTaxes Jun 10 at 12:19
1  
@QPaysTaxes True. However, the danger is that the hash then always creates and returns an empty array, if you access a non-existent key. This could be confusing if you afterward expect result["user_5"] to be nil, because you'll get an empty array instead. And if you afterward check result.keys you'll also see a "user_5" key in there. In other words, you can't quite rely on the hash telling you if a user is present in it or not - at least not in the usual way of nil == false. –  Flambino Jun 10 at 13:09
    
Thank you very much. need bit more explanation for you answers. it will really me. –  Ram Kumar Jun 10 at 13:25
    
Flambino, re @QPaysTaxes suggestion, isn't there a lesson here: to determine if a hash h has a key k, one should always use h.key?(k) (or h.has_key?(k) or h.include?(k)), rather than h[k]? –  Cary Swoveland Jun 13 at 5:35
1  
@CarySwoveland Using key? and its ilk is certainly safer and sidesteps any pitfalls here. Still, if this was wrapped in a method and part of some API, I wouldn't expect that method to return anything except a plain Hash. Docs could make that clear of course, but "principle of least surprise" and all that. Besides, I'm not a huge fan of how it, in this case, creates empty arrays for missing keys, when the expectation (that word again) would be a hash of only non-empty arrays. So my preference is to stick to ||=, though either thing's valid. –  Flambino Jun 13 at 8:49
up vote 1 down vote

Another way uses the form of Hash#update (aka merge) that uses a block to determine the values of keys that are present in both hashes being merged:

a.each_with_object({}) do |(name,id),h|
  h.update(name=>[id]) { |_,oid,nid| oid.concat(nid) }
end
  #=> {"user_3"=>[765], "user_1"=>[2, 1, 3], "user_2"=>[124, 223, 334]}

Let's see what's happening here:

e = a.each_with_object({})
  #=> #<Enumerator: [["user_3", 765], ["user_1", 2], ["user_1", 1],
  #                  ["user_2", 124], ["user_1", 3], ["user_2", 223],
  #                  ["user_2", 334]]:each_with_object({})>

The first element of e is passed to each_with_object's block and the block calculation is performed:

(name,id),h = e.next
     #=> [["user_3", 765], {}] 
name #=> "user_3" 
id   #=> 765 
h    #=> {} 
h.update(name=>[id])
     #=> {}.update("user_3"=>[765]) 
     #=> {"user_3"=>[765]}

update's block is not used here because the key "user_3" is not present in h. The next value of the enumerator is now passed to the block:

(name,id),h = e.next
  #=> [["user_1", 2], {"user_3"=>[765]}] 
h.update(name=>[id])
  #=> {"user_3"=>[765]}.update("user_1"=>[2])
  #=> {"user_3"=>[765], "user_1"=>[2]}

Again, the block is not used because h does not have a key "user_1". The third element of e is passed to the block:

(name,id),h = e.next
  #=> [["user_1", 1], {"user_3"=>[765], "user_1"=>[2]}] 
h.update(name=>[id])
  #=> {"user_3"=>[765], "user_1"=>[2]}.update("user_1"=>[1])

This time, both hashes being merged have the key "user_1", so the block is called up to determine the value of that key:

{ |key,oid,nid| oid.concat(nid) }
  #=> { |"user_1",[2],[1]| [2].concat([1]) }
  #=> [2,1]

so now:

h #=> {"user_3"=>[765], "user_1"=>[2, 1]}

I have drawn attention to the fact that the argument key was not used in this block calculation by replacing that variable with an underscore (which is indeed a variable). Some Rubiests might write _key instead.

The remaining calculations are similar.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.