HiveBrain v1.2.0
Get Started
← Back to all entries
patternrubyMinor

Guess the presents someone will get by comparing wishlist attributes

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thecomparingsomeonewishlistattributeswillgetpresentsguess

Problem

#
# Guess the presents someone will get
# We compare a `wishlist` with many criteria to the `presents` and return an array with the names of the matching products. 
#
def guess_gifts(wishlist, presents)

  wishlist.map do |w|
    w[:name] if presents.include? w.reject { |key| key == :name }
  end.compact.uniq

end

guess_gifts([

{:name => "mini puzzle", :size => "small", :clatters => "yes", :weight => "light"},
{:name => "toy car", :size => "medium", :clatters => "a bit", :weight => "medium"},
{:name => "card game", :size => "small", :clatters => "no", :weight => "light"}

],
[

{:size => "medium", :clatters => "a bit", :weight => "medium"},
{:size => "small", :clatters => "yes", :weight => "light"}

])


In particular, I'd like to know whether w.reject { |key| key == :name } is the best way to remove the :name item. Is there any shorter or faster way to do so? I am often very surprised with Ruby capacity to shorten processes, so I'm wondering.

Solution

It's nice that you were able to implement guess_gifts as a single expression.

I don't recommend writing it that way, though. For every item in the wishlist, you will scan the entire presents list for a match. Wouldn't it be nice to just do a lookup instead? You could do such a lookup if the wishlist were in a more convenient format.

def guess_gifts(wishlist, presents)
  # Transform the wishlist, mapping attributes to a list of names
  wishmap = Hash.new { |hash, attributes| hash[attributes] = [] }
  wishlist.each { |w| wishmap[w.reject { |k| k == :name }].push(w[:name]) }

  # Then just do the lookups
  presents.flat_map { |attributes| wishmap[attributes] }
end


Another benefit is that we're doing a map on presents rather than on wishlist, so I think we might not need to call uniq on the results.

Code Snippets

def guess_gifts(wishlist, presents)
  # Transform the wishlist, mapping attributes to a list of names
  wishmap = Hash.new { |hash, attributes| hash[attributes] = [] }
  wishlist.each { |w| wishmap[w.reject { |k| k == :name }].push(w[:name]) }

  # Then just do the lookups
  presents.flat_map { |attributes| wishmap[attributes] }
end

Context

StackExchange Code Review Q#125260, answer score: 2

Revisions (0)

No revisions yet.