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 & array2                unioned_users = array1 | array2                if unioned_users.length > 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]enddata = '[{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] << child[user]    endendall_activities = Set.newjaccard_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]] > 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 \nendOutput: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]]
Item-based collaborative filtering
ruby;clustering
It appears, that these lines of code are junk you forgot to remove:unless child[activities].include?(generic_activity)  Instead of accumulating set, just use users_for_activities.keys.Replace:child[activities].each do |user_activity|    all_activities.each do |generic_activity|        if user_activity != generic_activitywith:child[activities].product(all_activities - child[activities]) do |user_activity, generic_activity|that saves you two levels of indentation.The same trick here to replace:users_for_activities.each do |key1, array1|    users_for_activities.each do |key2, array2|        if key1 != key2with:users_for_activities.to_a.permutation(2) do |(key1, array1), (key2, array2)|You may use .empty? instead of .length > 0 or this:if 0 == total_users = array1.length + array2.length    jaccard_coefficient_hash[[key1, key2]] = 0else    jaccard_coefficient_hash[[key1, key2]] = intersected_users.length.fdiv(total_users)endSince recommendations can't be negative, you may get rid of another if because of its awful code length by defaulting the result to zero in this way:activities_to_recommend[generic_activity] = [    activities_to_recommend[generic_activity] || 0,    jaccard_coefficient_hash[[user_activity, generic_activity]]].maxor this:activities_to_recommend = Hash.new 0By changing loops nesting order we can start using Array#max and do a step to get rid of imperativity here, since we stop walk randomly through activities_to_recommend:parsed_data.each do |child|    activities_to_recommend = Hash.new 0    (users_for_activities.keys - child[activities]).each do |generic_activity|        activities_to_recommend[generic_activity] =            child[activities].map{ |user_activity|                jaccard_coefficient_hash[[user_activity, generic_activity]]            }.max || 0    end    puts User #{child[user]}: #{activities_to_recommend.sort_by{ |k,v| -v }}end(optional) But that getting rid of imperativity isn't too nice -- most people won't like this:activities_to_recommend = (users_for_activities.keys - child[activities]).map do |generic_activity| [    generic_activity,    child[activities].map{ |user_activity|        jaccard_coefficient_hash[[user_activity, generic_activity]]    }.max || 0] end(optional) If you try to rewrite users_for_activities creation into functional and method-chaining way, you'll get this controversial thing:users_for_activities = parsed_data.flat_map{ |child|    child[activities].map{ |activity| [activity, child[user]] }}.group_by(&:first).tap{ |h| h.each{ |k,v| v.map! &:last } }(optional) Hash with array as a key, rather than simple scalar, like an Integer, can have sudden collisions, but since your array items are Strings, you probably can forget about that.