From bcc9fd97f59ff08951a374498d828150876f1bd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Mon, 8 Apr 2024 19:27:31 +0200 Subject: [PATCH] Revert "Extract class to manage GeozoneStats" Back in commit 383909e16, we said: > Even if this class looks very simple now, we're trying a few things > related to these stats. Having a class for it makes future changes > easier and, if there weren't any future changes, at least it makes > current experiments easier. Since there haven't been any changes in the last 5 years and we've found cases where using the GeozoneStats class results in a slightly worse performance, we're removing this class. The code is now a bit easier to read, and is consistent with the way we calculate participants by age. This reverts commit 383909e16. --- app/lib/geozone_stats.rb | 24 --------------- app/lib/percentage_calculator.rb | 7 ----- app/models/concerns/statisticable.rb | 18 ++++++------ spec/lib/geozone_stats_spec.rb | 44 ---------------------------- 4 files changed, 9 insertions(+), 84 deletions(-) delete mode 100644 app/lib/geozone_stats.rb delete mode 100644 app/lib/percentage_calculator.rb delete mode 100644 spec/lib/geozone_stats_spec.rb diff --git a/app/lib/geozone_stats.rb b/app/lib/geozone_stats.rb deleted file mode 100644 index 6b7971610..000000000 --- a/app/lib/geozone_stats.rb +++ /dev/null @@ -1,24 +0,0 @@ -class GeozoneStats - attr_reader :geozone, :participants - - def initialize(geozone, participants) - @geozone = geozone - @participants = participants - end - - def geozone_participants - participants.where(geozone: geozone) - end - - def name - geozone.name - end - - def count - geozone_participants.count - end - - def percentage - PercentageCalculator.calculate(count, participants.count) - end -end diff --git a/app/lib/percentage_calculator.rb b/app/lib/percentage_calculator.rb deleted file mode 100644 index bb8f3d355..000000000 --- a/app/lib/percentage_calculator.rb +++ /dev/null @@ -1,7 +0,0 @@ -module PercentageCalculator - def self.calculate(fraction, total) - return 0.0 if total.zero? - - (fraction * 100.0 / total).round(3) - end -end diff --git a/app/models/concerns/statisticable.rb b/app/models/concerns/statisticable.rb index 53533b4ed..690cdecf5 100644 --- a/app/models/concerns/statisticable.rb +++ b/app/models/concerns/statisticable.rb @@ -114,19 +114,23 @@ module Statisticable end def participants_by_geozone - geozone_stats.to_h do |stats| + geozones.to_h do |geozone| + count = participants.where(geozone: geozone).count + [ - stats.name, + geozone.name, { - count: stats.count, - percentage: stats.percentage + count: count, + percentage: calculate_percentage(count, total_participants) } ] end end def calculate_percentage(fraction, total) - PercentageCalculator.calculate(fraction, total) + return 0.0 if total.zero? + + (fraction * 100.0 / total).round(3) end def version @@ -178,10 +182,6 @@ module Statisticable Geozone.order("name") end - def geozone_stats - geozones.map { |geozone| GeozoneStats.new(geozone, participants) } - end - def range_description(start, finish) if finish > 200 I18n.t("stats.age_more_than", start: start) diff --git a/spec/lib/geozone_stats_spec.rb b/spec/lib/geozone_stats_spec.rb deleted file mode 100644 index 570315406..000000000 --- a/spec/lib/geozone_stats_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -require "rails_helper" - -describe GeozoneStats do - let(:winterfell) { create(:geozone, name: "Winterfell") } - let(:riverlands) { create(:geozone, name: "Riverlands") } - - describe "#name" do - let(:stats) { GeozoneStats.new(winterfell, []) } - - it "returns the geozone name" do - expect(stats.name).to eq "Winterfell" - end - end - - describe "#count" do - before do - 2.times { create(:user, geozone: winterfell) } - 1.times { create(:user, geozone: riverlands) } - end - - let(:winterfell_stats) { GeozoneStats.new(winterfell, User.all) } - let(:riverlands_stats) { GeozoneStats.new(riverlands, User.all) } - - it "counts participants from the geozone" do - expect(winterfell_stats.count).to eq 2 - expect(riverlands_stats.count).to eq 1 - end - end - - describe "#percentage" do - before do - 2.times { create(:user, geozone: winterfell) } - 1.times { create(:user, geozone: riverlands) } - end - - let(:winterfell_stats) { GeozoneStats.new(winterfell, User.all) } - let(:riverlands_stats) { GeozoneStats.new(riverlands, User.all) } - - it "calculates percentage relative to the amount of participants" do - expect(winterfell_stats.percentage).to eq 66.667 - expect(riverlands_stats.percentage).to eq 33.333 - end - end -end