Merge pull request 'Use group affinities in discomfort calculator' (#215) from discomfort-calculation-revamp into main

Reviewed-on: #215

app/controllers/affinities_controller.rb

@@ -36,17 +36,21 @@ class AffinitiesController < ApplicationController
   end
 
   def default
+    hierarchy = AffinityGroupsHierarchy.new
+
     for_each_group do |group_id|
-      Tables::DiscomfortCalculator.cohesion_discomfort(id_a: @group.id, id_b: group_id).to_f
+      hierarchy.default_discomfort(@group.id, group_id).to_f
     end
   end
 
   def reset
+    hierarchy = AffinityGroupsHierarchy.new
+
     affinities = Group.pluck(:id).combination(2).map do |(group_a_id, group_b_id)|
       {
         group_a_id:,
         group_b_id:,
-        discomfort: Tables::DiscomfortCalculator.cohesion_discomfort(id_a: group_a_id, id_b: group_b_id).to_f
+        discomfort: hierarchy.default_discomfort(group_a_id, group_b_id).to_f
       }
     end
 

app/services/affinity_groups_hierarchy.rb

@@ -5,7 +5,7 @@
 # frozen_string_literal: true
 
 class AffinityGroupsHierarchy < Array
-  include Singleton
+  DEFAULT_DISCOMFORT = 1
 
   def initialize
     super
@@ -16,6 +16,9 @@ class AffinityGroupsHierarchy < Array
 
       hydrate(group)
     end
+
+    load_discomforts
+    freeze
   end
 
   def find(id)
@@ -37,8 +40,35 @@ class AffinityGroupsHierarchy < Array
     @references[id_a].distance_to_common_ancestor(@references[id_b])
   end
 
+  def discomfort(id_a, id_b)
+    return 0 if id_a == id_b
+
+    @discomforts[uuid_to_int(id_a) + uuid_to_int(id_b)] || DEFAULT_DISCOMFORT
+  end
+
+  def default_discomfort(id_a, id_b)
+    return 0 if id_a == id_b
+
+    dist = distance(id_a, id_b)
+
+    return DEFAULT_DISCOMFORT if dist.nil?
+
+    Rational(dist, dist + 1)
+  end
+
   private
 
+  def load_discomforts
+    @load_discomforts ||= GroupAffinity.pluck(:group_a_id, :group_b_id,
+                                              :discomfort).each_with_object({}) do |(id_a, id_b, discomfort), acc|
+      acc[uuid_to_int(id_a) + uuid_to_int(id_b)] = discomfort
+    end
+  end
+
+  def uuid_to_int(uuid)
+    uuid.gsub('-', '').hex
+  end
+
   def hydrate(group)
     group.children.each do |child|
       register_child(group.id, child.id)

app/services/tables/discomfort_calculator.rb

@@ -6,19 +6,10 @@
 
 module Tables
   class DiscomfortCalculator
-    class << self
+    private attr_reader :table, :hierarchy
-      def cohesion_discomfort(id_a:, id_b:)
+    def initialize(table:, hierarchy: AffinityGroupsHierarchy.new)
-        distance = AffinityGroupsHierarchy.instance.distance(id_a, id_b)
-
-        return 1 if distance.nil?
-
-        Rational(distance, distance + 1)
-      end
-    end
-
-    private attr_reader :table
-    def initialize(table:)
       @table = table
+      @hierarchy = hierarchy
     end
 
     def calculate
@@ -59,7 +50,7 @@ module Tables
     #
     def cohesion_discomfort
       table.map(&:group_id).tally.to_a.combination(2).sum do |(a, count_a), (b, count_b)|
-        count_a * count_b * self.class.cohesion_discomfort(id_a: a, id_b: b)
+        count_a * count_b * hierarchy.discomfort(a, b)
       end
     end
   end

app/services/tables/distribution.rb

@@ -13,6 +13,7 @@ module Tables
     def initialize(min_per_table:, max_per_table:)
       @min_per_table = min_per_table
       @max_per_table = max_per_table
+      @hierarchy = AffinityGroupsHierarchy.new
       @tables = []
     end
 
@@ -35,12 +36,6 @@ module Tables
       "#{@tables.count} tables, discomfort: #{discomfort}"
     end
 
-    def pretty_print
-      @tables.map.with_index do |table, i|
-        "Table #{i + 1} (#{table.count} ppl): (#{local_discomfort(table)}) #{table.map(&:name).join(', ')}"
-      end.join("\n")
-    end
-
     def deep_dup
       self.class.new(min_per_table: @min_per_table, max_per_table: @max_per_table).tap do |new_distribution|
         new_distribution.tables = @tables.map(&:dup)
@@ -68,7 +63,7 @@ module Tables
     private
 
     def local_discomfort(table)
-      table.discomfort ||= DiscomfortCalculator.new(table:).calculate
+      table.discomfort ||= DiscomfortCalculator.new(table:, hierarchy:).calculate
     end
   end
 end

spec/services/tables/discomfort_calculator_spec.rb

@@ -75,159 +75,5 @@ module Tables
         it { expect(calculator.send(:table_size_penalty)).to eq(10) }
       end
     end
-
-    describe '#cohesion_discomfort' do
-      before do
-        # Overridden in each test except trivial cases
-        allow(AffinityGroupsHierarchy.instance).to receive(:distance).and_call_original
-
-        %w[family friends work school].each do |group|
-          allow(AffinityGroupsHierarchy.instance).to receive(:distance).with(group, group).and_return(0)
-        end
-
-        allow(AffinityGroupsHierarchy.instance).to receive(:distance).with(family.id, friends.id).and_return(nil)
-        allow(AffinityGroupsHierarchy.instance).to receive(:distance).with(friends.id, work.id).and_return(1)
-        allow(AffinityGroupsHierarchy.instance).to receive(:distance).with(family.id, work.id).and_return(2)
-        allow(AffinityGroupsHierarchy.instance).to receive(:distance).with(family.id, school.id).and_return(3)
-        allow(AffinityGroupsHierarchy.instance).to receive(:distance).with(friends.id, school.id).and_return(4)
-        allow(AffinityGroupsHierarchy.instance).to receive(:distance).with(work.id, school.id).and_return(5)
-      end
-
-      context 'when the table contains just two guests' do
-        context 'when they belong to the same group' do
-          let(:table) { create_list(:guest, 2, group: family) }
-
-          it { expect(calculator.send(:cohesion_discomfort)).to eq(0) }
-        end
-
-        context 'when they belong to completely unrelated groups' do
-          let(:table) do
-            [
-              create(:guest, group: family),
-              create(:guest, group: friends)
-            ]
-          end
-
-          it { expect(calculator.send(:cohesion_discomfort)).to eq(1) }
-        end
-
-        context 'when they belong to groups at a distance of 1' do
-          let(:table) do
-            [
-              create(:guest, group: friends),
-              create(:guest, group: work)
-            ]
-          end
-
-          it { expect(calculator.send(:cohesion_discomfort)).to eq(0.5) }
-        end
-
-        context 'when they belong to groups at a distance of 2' do
-          let(:table) do
-            [
-              create(:guest, group: family),
-              create(:guest, group: work)
-            ]
-          end
-
-          it { expect(calculator.send(:cohesion_discomfort)).to eq(Rational(2, 3)) }
-        end
-
-        context 'when they belong to groups at a distance of 3' do
-          let(:table) do
-            [
-              create(:guest, group: family),
-              create(:guest, group: school)
-            ]
-          end
-
-          it { expect(calculator.send(:cohesion_discomfort)).to eq(Rational(3, 4)) }
-        end
-      end
-
-      context 'when the table contains three guests' do
-        let(:table) do
-          [
-            create(:guest, group: family),
-            create(:guest, group: friends),
-            create(:guest, group: work)
-          ]
-        end
-
-        it 'returns the sum of the penalties for each pair of guests' do
-          expect(calculator.send(:cohesion_discomfort)).to eq(1 + Rational(1, 2) + Rational(2, 3))
-        end
-      end
-
-      context 'when the table contains four guests of different groups' do
-        let(:table) do
-          [
-            create(:guest, group: family),
-            create(:guest, group: friends),
-            create(:guest, group: work),
-            create(:guest, group: school)
-          ]
-        end
-
-        it 'returns the sum of the penalties for each pair of guests' do
-          expect(calculator.send(:cohesion_discomfort))
-            .to eq(1 + Rational(1, 2) + Rational(2, 3) + Rational(3, 4) + Rational(4, 5) + Rational(5, 6))
-        end
-      end
-
-      context 'when the table contains four guests of two evenly split groups' do
-        let(:table) do
-          [
-            create_list(:guest, 2, group: family),
-            create_list(:guest, 2, group: friends)
-          ].flatten
-        end
-
-        it 'returns the sum of the penalties for each pair of guests' do
-          expect(calculator.send(:cohesion_discomfort)).to eq(4)
-        end
-      end
-
-      context 'when the table contains six guests of two unevenly split groups' do
-        let(:table) do
-          [
-            create_list(:guest, 2, group: family),
-            create_list(:guest, 4, group: friends)
-          ].flatten
-        end
-
-        it 'returns the sum of the penalties for each pair of guests' do
-          expect(calculator.send(:cohesion_discomfort)).to eq(8)
-        end
-      end
-
-      context 'when the table contains six guests of three evenly split groups' do
-        let(:table) do
-          [
-            create_list(:guest, 2, group: family),
-            create_list(:guest, 2, group: friends),
-            create_list(:guest, 2, group: work)
-          ].flatten
-        end
-
-        it 'returns the sum of the penalties for each pair of guests' do
-          expect(calculator.send(:cohesion_discomfort)).to eq((4 * 1) + (4 * Rational(1, 2)) + (4 * Rational(2, 3)))
-        end
-      end
-
-      context 'when the table contains six guests of three unevenly split groups' do
-        let(:table) do
-          [
-            create_list(:guest, 3, group: family),
-            create_list(:guest, 2, group: friends),
-            create_list(:guest, 1, group: work)
-          ].flatten
-        end
-
-        it 'returns the sum of the penalties for each pair of guests' do
-          expect(calculator.send(:cohesion_discomfort)).to eq((6 * 1) + (2 * Rational(1, 2)) + (3 * Rational(2, 3)))
-        end
-      end
-    end
   end
 end