Item-based collaborative filtering I wrote the following code using Ruby to compute item-based collaborative filtering (using Jaccard coefficient of similarity). 

I would appreciate feedback about any potential issues with the code and tips to make the code better comply with the best practices.

require 'json'
require 'set'

def compute_jaccard_coefficients(users_for_activities)
    jaccard_coefficient_hash = {}
    all_activities = Set.new
    users_for_activities.each do |key1, array1|
        users_for_activities.each do |key2, array2|
            if key1 != key2
                all_activities.add key1
                all_activities.add key2
                intersected_users = array1 &amp; array2
                unioned_users = array1 | array2
                if unioned_users.length &gt; 0
                    jaccard_coefficient_hash[[key1, key2]] = intersected_users.length.fdiv(unioned_users.length)
                    else
                    jaccard_coefficient_hash[[key1, key2]] = 0
                end
            end
        end
    end
    return [jaccard_coefficient_hash, all_activities]
end

data = '[
{"user": "1", "activities": ["running", "swimming", "rowing"]},
{"user": "2", "activities": ["tennis", "rowing"]},
{"user": "3", "activities": ["swimming", "running"]},
{"user": "4", "activities": ["tennis", "swimming"]}
]'

parsed_data = JSON.parse(data)

users_for_activities = Hash.new{|h,k| h[k] = []}

parsed_data.each do |child|
    child["activities"].each do |activity|
        users_for_activities[activity] &lt;&lt; child["user"]
    end
end

all_activities = Set.new
jaccard_coefficient_hash, all_activities = compute_jaccard_coefficients(users_for_activities)

parsed_data.each do |child|
    activities_to_recommend = {}
    child["activities"].each do |user_activity|
        all_activities.each do |generic_activity|
            if user_activity != generic_activity
                if activities_to_recommend.has_key?(generic_activity)
                    if jaccard_coefficient_hash[[user_activity, generic_activity]] &gt; activities_to_recommend[generic_activity]
                        unless child["activities"].include?(generic_activity)
                            activities_to_recommend[generic_activity] = jaccard_coefficient_hash[[user_activity, generic_activity]]
                        end
                    end
                    else
                    unless child["activities"].include?(generic_activity)
                        activities_to_recommend[generic_activity] = jaccard_coefficient_hash[[user_activity, generic_activity]]
                    end
                end
            end
        end
    end
    print "User " + child["user"] + ": "
    print activities_to_recommend.sort_by {|k,v| v}.reverse
    print "\n"
end


Output:




User 1: [["tennis", 0.3333333333333333]]
User 2: [["running", 0.3333333333333333], ["swimming", 0.25]]
User 3: [["rowing", 0.3333333333333333], ["tennis", 0.25]]
User 4: [["running", 0.6666666666666666], ["rowing", 0.3333333333333333]]
Stuff I noticed:


Use next to skip an each-iteration instead of wrapping everything in an if statement
No need for Set. A hash's keys are already a set, so in your compute_jaccard_coefficients method you could just call all_activties = users_for_activities.keys and done. From the raw data, you could do all_activities = parsed_data.map { |user| user['activities'] }.flatten.uniq
I'd absolutely avoid returning a tuple from the compute_jaccard_coefficients method. As illustrated above, getting all the activities isn't difficult; you can do that anywhere. And the method itself doesn't use the set for anything; creating the set is just something it also does, even though it shouldn't be its responsibility. Your code definitely shouldn't rely on a method just randomly doing something 
Since your method is checking combinations of activities, you might as well use Array#combination. Give it an argument of 2 and it'll give you an array of pairs activities. Note that it'll also give only unique combinations - i.e. you'll get a pair like ["tennis", "swimming"], but it'll skip ["swimming", "tennis"] since that's the same combination.
Favor methods like any? (or better - as Naklion points out in the comments - empty?) over checking an array's length with arr.count &gt; 0


With the above in mind, the method can be written as

def compute_jaccard_coefficients(hash)
  coefficients = hash.keys.combination(2).map do |a, b|
    union = hash[a] | hash[b]
    weight = union.empty? ? 0 : (hash[a] &amp; hash[b]).count / union.count.to_f
    [[a, b], weight]
  end
  Hash[coefficients]
end


Note that this implementation also gives you a cleaner result (no duplicate combinations) in fewer iterations.

I'm out of time right now, but I'll take a look at the rest of the code later.



Update: It's now "later", and Naklion has provided a very nice answer. Still, I'll give it a go too.

I used combination above, whereas Naklion used permutation. The latter gives an identical result to the original code (i.e. the coefficients hash will have keys for both tennis/rowing and rowing/tennis). Using combination you'll only get unique combinations, and thus fewer coefficient calculations, but lookup gets a bit more complex (which I wasn't considering when just looking at the Jaccard method). Still, since Naklion used permutations, I'll stick to combinations.

One addition, though, is to use Set (after all) to create the keys of the Jaccard hash:

def compute_jaccard_coefficients(hash)
  coefficients = hash.keys.combination(2).map do |a, b|
    union = hash[a] | hash[b]
    weight = union.empty? ? 0 : (hash[a] &amp; hash[b]).count / union.count.to_f
    [Set[a, b], weight]
  end
  Hash[coefficients]
end


This allows us to look up weights without worrying bout the order of the key's elements, since Set[a, b] == Set[b, a].

Naklion provided a nice way (#9 on the list) to transpose the parsed_data from user-indexed activities to activity-indexed users, so I'll skip over that.

So starting with this (and the method above)

all_activities = users_for_activities.keys
coefficients = compute_jaccard_coefficients(users_for_activities)


you can do this

parsed_data.each do |user|
  possibles = all_activities - user['activities'] # activities the user doesn't have already
  recommendations = user['activities'].product(possibles).map do |a, b|
    # a is the user's existing activity, b is the recommended activity
    key = Set[a, b]
    [b, coefficients[key]] if coefficients[key] &gt; 0
  end.compact.sort_by(&amp;:last).reverse

  puts "User #{user[:user]}: #{recommendations}"
end