From 7319ef73451282b291f1acd81b4781d027a321b4 Mon Sep 17 00:00:00 2001
From: Andy Brett <andy@andybrett.com>
Date: Mon, 8 Mar 2021 09:23:23 -0800
Subject: [PATCH] use better method and class names

---
 app/serializers/api/variant_serializer.rb     |   8 +-
 app/services/unit_prices.rb                   |  35 ++++++
 .../variant_units/option_value_namer.rb       |   2 +-
 app/services/variant_units/unit_prices.rb     |  38 -------
 .../variant_units/weights_and_measures.rb     |  60 ----------
 app/services/weights_and_measures.rb          |  58 ++++++++++
 spec/services/unit_prices_spec.rb             | 105 +++++++++++++++++
 .../variant_units/unit_prices_spec.rb         | 107 ------------------
 .../weights_and_measures_spec.rb              |  89 ---------------
 spec/services/weights_and_measures_spec.rb    |  87 ++++++++++++++
 10 files changed, 290 insertions(+), 299 deletions(-)
 create mode 100644 app/services/unit_prices.rb
 delete mode 100644 app/services/variant_units/unit_prices.rb
 delete mode 100644 app/services/variant_units/weights_and_measures.rb
 create mode 100644 app/services/weights_and_measures.rb
 create mode 100644 spec/services/unit_prices_spec.rb
 delete mode 100644 spec/services/variant_units/unit_prices_spec.rb
 delete mode 100644 spec/services/variant_units/weights_and_measures_spec.rb
 create mode 100644 spec/services/weights_and_measures_spec.rb

diff --git a/app/serializers/api/variant_serializer.rb b/app/serializers/api/variant_serializer.rb
index 60ae1f441d..ecbfc66dc7 100644
--- a/app/serializers/api/variant_serializer.rb
+++ b/app/serializers/api/variant_serializer.rb
@@ -41,16 +41,16 @@ class Api::VariantSerializer < ActiveModel::Serializer
   end
 
   def unit_price_price
-    price_with_fees / unit_prices.denominator
+    price_with_fees / unit_price.denominator
   end
 
   def unit_price_unit
-    unit_prices.unit
+    unit_price.unit
   end
 
   private
 
-  def unit_prices
-    @unit_prices ||= VariantUnits::UnitPrices.new(object)
+  def unit_price
+    @unit_price ||= UnitPrice.new(object)
   end
 end
diff --git a/app/services/unit_prices.rb b/app/services/unit_prices.rb
new file mode 100644
index 0000000000..0b28c973d5
--- /dev/null
+++ b/app/services/unit_prices.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+class UnitPrice
+  def initialize(variant)
+    @variant = variant
+    @product = variant.product
+  end
+
+  def denominator
+    # catches any case where unit is not kg, lb, or L.
+    return @variant.unit_value if @product&.variant_unit == "items"
+
+    case unit
+    when "lb"
+      @variant.unit_value / 453.6
+    when "kg"
+      @variant.unit_value / 1000
+    else # Liters
+      @variant.unit_value
+    end
+  end
+
+  def unit
+    return "lb" if WeightsAndMeasures.new(@variant).system == "imperial"
+
+    case @product&.variant_unit
+    when "weight"
+      "kg"
+    when "volume"
+      "L"
+    else
+      @product.variant_unit_name.presence || I18n.t("item")
+    end
+  end
+end
diff --git a/app/services/variant_units/option_value_namer.rb b/app/services/variant_units/option_value_namer.rb
index 934a873b78..9d95f58f2e 100644
--- a/app/services/variant_units/option_value_namer.rb
+++ b/app/services/variant_units/option_value_namer.rb
@@ -62,7 +62,7 @@ module VariantUnits
     end
 
     def scale_for_unit_value
-      VariantUnits::WeightsAndMeasures.new(@variant).scale_for_unit_value
+      WeightsAndMeasures.new(@variant).scale_for_unit_value
     end
 
     def pluralize(unit_name, count)
diff --git a/app/services/variant_units/unit_prices.rb b/app/services/variant_units/unit_prices.rb
deleted file mode 100644
index 23ac3a079b..0000000000
--- a/app/services/variant_units/unit_prices.rb
+++ /dev/null
@@ -1,38 +0,0 @@
-# frozen_string_literal: true
-
-module VariantUnits
-  class UnitPrices
-    def initialize(variant)
-      @variant = variant
-      @product = variant.product
-    end
-
-    def denominator
-      # catches any case where unit is not kg, lb, or L.
-      return @variant.unit_value if @product&.variant_unit == "items"
-
-      case unit
-      when "lb"
-        @variant.unit_value / 453.6
-      when "kg"
-        @variant.unit_value / 1000
-      else # Liters
-        @variant.unit_value
-      end
-    end
-
-    def unit
-      return "lb" if VariantUnits::WeightsAndMeasures.new(@variant).
-        system_of_measurement == "imperial"
-
-      case @product&.variant_unit
-      when "weight"
-        "kg"
-      when "volume"
-        "L"
-      else
-        @product.variant_unit_name.presence || I18n.t("item")
-      end
-    end
-  end
-end
diff --git a/app/services/variant_units/weights_and_measures.rb b/app/services/variant_units/weights_and_measures.rb
deleted file mode 100644
index 37b2b17368..0000000000
--- a/app/services/variant_units/weights_and_measures.rb
+++ /dev/null
@@ -1,60 +0,0 @@
-# frozen_string_literal: true
-
-module VariantUnits
-  class WeightsAndMeasures
-    def initialize(variant)
-      @variant = variant
-      @units = UNITS
-    end
-
-    def scale_for_unit_value
-      largest_unit = find_largest_unit(scales_for_variant_unit, system_of_measurement)
-      return [nil, nil] unless largest_unit
-
-      [largest_unit[0], largest_unit[1]["name"]]
-    end
-
-    def system_of_measurement
-      scales = scales_for_variant_unit
-      return "custom" unless product_scale = @variant.product.variant_unit_scale
-
-      scales[product_scale.to_f]['system']
-    end
-
-    private
-
-    UNITS = {
-      'weight' => {
-        1.0 => { 'name' => 'g', 'system' => 'metric' },
-        28.35 => { 'name' => 'oz', 'system' => 'imperial' },
-        453.6 => { 'name' => 'lb', 'system' => 'imperial' },
-        1000.0 => { 'name' => 'kg', 'system' => 'metric' },
-        1_000_000.0 => { 'name' => 'T', 'system' => 'metric' }
-      },
-      'volume' => {
-        0.001 => { 'name' => 'mL', 'system' => 'metric' },
-        1.0 => { 'name' => 'L', 'system' => 'metric' },
-        1000.0 => { 'name' => 'kL', 'system' => 'metric' }
-      }
-    }.freeze
-
-    def scales_for_variant_unit
-      @units[@variant.product.variant_unit]
-    end
-
-    # Find the largest available and compatible unit where unit_value comes
-    #   to >= 1 when expressed in it.
-    # If there is none available where this is true, use the smallest available unit.
-    def find_largest_unit(scales, product_scale_system)
-      return nil unless scales
-
-      largest_unit = scales.select { |scale, unit_info|
-        unit_info['system'] == product_scale_system &&
-          @variant.unit_value / scale >= 1
-      }.max
-      return scales.first if largest_unit.nil?
-
-      largest_unit
-    end
-  end
-end
diff --git a/app/services/weights_and_measures.rb b/app/services/weights_and_measures.rb
new file mode 100644
index 0000000000..eaf66b3ec4
--- /dev/null
+++ b/app/services/weights_and_measures.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+class WeightsAndMeasures
+  def initialize(variant)
+    @variant = variant
+    @units = UNITS
+  end
+
+  def scale_for_unit_value
+    largest_unit = find_largest_unit(scales_for_variant_unit, system)
+    return [nil, nil] unless largest_unit
+
+    [largest_unit[0], largest_unit[1]["name"]]
+  end
+
+  def system
+    scales = scales_for_variant_unit
+    return "custom" unless product_scale = @variant.product.variant_unit_scale
+
+    scales[product_scale.to_f]['system']
+  end
+
+  private
+
+  UNITS = {
+    'weight' => {
+      1.0 => { 'name' => 'g', 'system' => 'metric' },
+      28.35 => { 'name' => 'oz', 'system' => 'imperial' },
+      453.6 => { 'name' => 'lb', 'system' => 'imperial' },
+      1000.0 => { 'name' => 'kg', 'system' => 'metric' },
+      1_000_000.0 => { 'name' => 'T', 'system' => 'metric' }
+    },
+    'volume' => {
+      0.001 => { 'name' => 'mL', 'system' => 'metric' },
+      1.0 => { 'name' => 'L', 'system' => 'metric' },
+      1000.0 => { 'name' => 'kL', 'system' => 'metric' }
+    }
+  }.freeze
+
+  def scales_for_variant_unit
+    @units[@variant.product.variant_unit]
+  end
+
+  # Find the largest available and compatible unit where unit_value comes
+  #   to >= 1 when expressed in it.
+  # If there is none available where this is true, use the smallest available unit.
+  def find_largest_unit(scales, product_scale_system)
+    return nil unless scales
+
+    largest_unit = scales.select { |scale, unit_info|
+      unit_info['system'] == product_scale_system &&
+        @variant.unit_value / scale >= 1
+    }.max
+    return scales.first if largest_unit.nil?
+
+    largest_unit
+  end
+end
diff --git a/spec/services/unit_prices_spec.rb b/spec/services/unit_prices_spec.rb
new file mode 100644
index 0000000000..2ea2d04b52
--- /dev/null
+++ b/spec/services/unit_prices_spec.rb
@@ -0,0 +1,105 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe UnitPrices do
+  subject { UnitPrices.new(variant) }
+  let(:variant) { Spree::Variant.new }
+  let(:product) { double }
+
+  before do
+    allow(variant).to receive(:product) { product }
+  end
+
+  describe "#unit" do
+    context "metric" do
+      before do
+        allow(product).to receive(:variant_unit_scale) { 1.0 }
+      end
+
+      it "returns kg for weight" do
+        allow(product).to receive(:variant_unit) { "weight" }
+        expect(subject.unit).to eq("kg")
+      end
+
+      it "returns L for volume" do
+        allow(product).to receive(:variant_unit) { "volume" }
+        expect(subject.unit).to eq("L")
+      end
+    end
+
+    context "imperial" do
+      it "returns lbs" do
+        allow(product).to receive(:variant_unit_scale) { 453.6 }
+        allow(product).to receive(:variant_unit) { "weight" }
+        expect(subject.unit).to eq("lb")
+      end
+    end
+
+    context "items" do
+      it "returns items if no unit is specified" do
+        allow(product).to receive(:variant_unit_name) { nil }
+        allow(product).to receive(:variant_unit_scale) { nil }
+        allow(product).to receive(:variant_unit) { "items" }
+        expect(subject.unit).to eq("item")
+      end
+
+      it "returns the unit if a unit is specified" do
+        allow(product).to receive(:variant_unit_name) { "bunch" }
+        allow(product).to receive(:variant_unit_scale) { nil }
+        allow(product).to receive(:variant_unit) { "items" }
+        expect(subject.unit).to eq("bunch")
+      end
+    end
+  end
+
+  describe "#denominator" do
+    context "metric" do
+      it "returns 0.5 for a 500g variant" do
+        allow(product).to receive(:variant_unit_scale) { 1.0 }
+        allow(product).to receive(:variant_unit) { "weight" }
+        variant.unit_value = 500
+        expect(subject.denominator).to eq(0.5)
+      end
+
+      it "returns 2 for a 2kg variant" do
+        allow(product).to receive(:variant_unit_scale) { 1000 }
+        allow(product).to receive(:variant_unit) { "weight" }
+        variant.unit_value = 2000
+        expect(subject.denominator).to eq(2)
+      end
+
+      it "returns 0.5 for a 500mL variant" do
+        allow(product).to receive(:variant_unit_scale) { 0.001 }
+        allow(product).to receive(:variant_unit) { "volume" }
+        variant.unit_value = 0.5
+        expect(subject.denominator).to eq(0.5)
+      end
+    end
+
+    context "imperial" do
+      it "returns 2 for a 2 pound variant" do
+        allow(product).to receive(:variant_unit_scale) { 453.6 }
+        allow(product).to receive(:variant_unit) { "weight" }
+        variant.unit_value = 2*453.6
+        expect(subject.denominator).to eq(2)
+      end
+    end
+
+    context "items" do
+      it "returns 1 if no unit is specified" do
+        allow(product).to receive(:variant_unit_scale) { nil }
+        allow(product).to receive(:variant_unit) { "items" }
+        variant.unit_value = 1
+        expect(subject.denominator).to eq(1)
+      end
+
+      it "returns 2 for multi-item units" do
+        allow(product).to receive(:variant_unit_scale) { nil }
+        allow(product).to receive(:variant_unit) { "items" }
+        variant.unit_value = 2
+        expect(subject.denominator).to eq(2)
+      end
+    end
+  end
+end
diff --git a/spec/services/variant_units/unit_prices_spec.rb b/spec/services/variant_units/unit_prices_spec.rb
deleted file mode 100644
index 089116ae3c..0000000000
--- a/spec/services/variant_units/unit_prices_spec.rb
+++ /dev/null
@@ -1,107 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-module VariantUnits
-  describe UnitPrices do
-    subject { UnitPrices.new(variant) }
-    let(:variant) { Spree::Variant.new }
-    let(:product) { double }
-
-    before do
-      allow(variant).to receive(:product) { product }
-    end
-
-    describe "#unit" do
-      context "metric" do
-        before do
-          allow(product).to receive(:variant_unit_scale) { 1.0 }
-        end
-
-        it "returns kg for weight" do
-          allow(product).to receive(:variant_unit) { "weight" }
-          expect(subject.unit).to eq("kg")
-        end
-
-        it "returns L for volume" do
-          allow(product).to receive(:variant_unit) { "volume" }
-          expect(subject.unit).to eq("L")
-        end
-      end
-
-      context "imperial" do
-        it "returns lbs" do
-          allow(product).to receive(:variant_unit_scale) { 453.6 }
-          allow(product).to receive(:variant_unit) { "weight" }
-          expect(subject.unit).to eq("lb")
-        end
-      end
-
-      context "items" do
-        it "returns items if no unit is specified" do
-          allow(product).to receive(:variant_unit_name) { nil }
-          allow(product).to receive(:variant_unit_scale) { nil }
-          allow(product).to receive(:variant_unit) { "items" }
-          expect(subject.unit).to eq("item")
-        end
-
-        it "returns the unit if a unit is specified" do
-          allow(product).to receive(:variant_unit_name) { "bunch" }
-          allow(product).to receive(:variant_unit_scale) { nil }
-          allow(product).to receive(:variant_unit) { "items" }
-          expect(subject.unit).to eq("bunch")
-        end
-      end
-    end
-
-    describe "#denominator" do
-      context "metric" do
-        it "returns 0.5 for a 500g variant" do
-          allow(product).to receive(:variant_unit_scale) { 1.0 }
-          allow(product).to receive(:variant_unit) { "weight" }
-          variant.unit_value = 500
-          expect(subject.denominator).to eq(0.5)
-        end
-
-        it "returns 2 for a 2kg variant" do
-          allow(product).to receive(:variant_unit_scale) { 1000 }
-          allow(product).to receive(:variant_unit) { "weight" }
-          variant.unit_value = 2000
-          expect(subject.denominator).to eq(2)
-        end
-
-        it "returns 0.5 for a 500mL variant" do
-          allow(product).to receive(:variant_unit_scale) { 0.001 }
-          allow(product).to receive(:variant_unit) { "volume" }
-          variant.unit_value = 0.5
-          expect(subject.denominator).to eq(0.5)
-        end
-      end
-
-      context "imperial" do
-        it "returns 2 for a 2 pound variant" do
-          allow(product).to receive(:variant_unit_scale) { 453.6 }
-          allow(product).to receive(:variant_unit) { "weight" }
-          variant.unit_value = 2*453.6
-          expect(subject.denominator).to eq(2)
-        end
-      end
-
-      context "items" do
-        it "returns 1 if no unit is specified" do
-          allow(product).to receive(:variant_unit_scale) { nil }
-          allow(product).to receive(:variant_unit) { "items" }
-          variant.unit_value = 1
-          expect(subject.denominator).to eq(1)
-        end
-
-        it "returns 2 for multi-item units" do
-          allow(product).to receive(:variant_unit_scale) { nil }
-          allow(product).to receive(:variant_unit) { "items" }
-          variant.unit_value = 2
-          expect(subject.denominator).to eq(2)
-        end
-      end
-    end
-  end
-end
diff --git a/spec/services/variant_units/weights_and_measures_spec.rb b/spec/services/variant_units/weights_and_measures_spec.rb
deleted file mode 100644
index f178aea09e..0000000000
--- a/spec/services/variant_units/weights_and_measures_spec.rb
+++ /dev/null
@@ -1,89 +0,0 @@
-# frozen_string_literal: true
-
-require 'spec_helper'
-
-module VariantUnits
-  describe WeightsAndMeasures do
-    subject { WeightsAndMeasures.new(variant) }
-    let(:variant) { Spree::Variant.new }
-    let(:product) { double }
-
-    before do
-      allow(variant).to receive(:product) { product }
-    end
-
-    describe "#system_of_measurement" do
-      context "weight" do
-        before do
-          allow(product).to receive(:variant_unit) { "weight" }
-        end
-
-        it "when scale is for a metric unit" do
-          allow(product).to receive(:variant_unit_scale) { 1.0 }
-          expect(subject.system_of_measurement).to eq("metric")
-        end
-
-        it "when scale is for an imperial unit" do
-          allow(product).to receive(:variant_unit_scale) { 28.35 }
-          expect(subject.system_of_measurement).to eq("imperial")
-        end
-      end
-
-      context "volume" do
-        it "when scale is for a metric unit" do
-          allow(product).to receive(:variant_unit) { "volume" }
-          allow(product).to receive(:variant_unit_scale) { 1.0 }
-          expect(subject.system_of_measurement).to eq("metric")
-        end
-      end
-
-      context "items" do
-        it "when scale is for items" do
-          allow(product).to receive(:variant_unit) { "items" }
-          allow(product).to receive(:variant_unit_scale) { nil }
-          expect(subject.system_of_measurement).to eq("custom")
-        end
-      end
-    end
-
-    describe "#scale_for_unit_value" do
-      context "weight" do
-        before do
-          allow(product).to receive(:variant_unit) { "weight" }
-        end
-
-        context "metric" do
-          it "for a unit value that should display in grams" do
-            allow(product).to receive(:variant_unit_scale) { 1.0 }
-            allow(variant).to receive(:unit_value) { 500 }
-            expect(subject.scale_for_unit_value).to eq([1.0, "g"])
-          end
-
-          it "for a unit value that should display in kg" do
-            allow(product).to receive(:variant_unit_scale) { 1.0 }
-            allow(variant).to receive(:unit_value) { 1500 }
-            expect(subject.scale_for_unit_value).to eq([1000.0, "kg"])
-          end
-        end
-      end
-
-      context "volume" do
-        it "for a unit value that should display in L" do
-          allow(product).to receive(:variant_unit) { "volume" }
-          allow(product).to receive(:variant_unit_scale) { 1.0 }
-          allow(variant).to receive(:unit_value) { 1500 }
-          expect(subject.scale_for_unit_value).to eq([1000, "kL"])
-        end
-      end
-
-      context "items" do
-        it "when scale is for items" do
-          allow(product).to receive(:variant_unit) { "items" }
-          allow(product).to receive(:variant_unit_scale) { nil }
-          allow(variant).to receive(:unit_value) { 4 }
-          expect(subject.scale_for_unit_value).to eq([nil, nil])
-        end
-      end
-    end
-  end
-end
diff --git a/spec/services/weights_and_measures_spec.rb b/spec/services/weights_and_measures_spec.rb
new file mode 100644
index 0000000000..616b6a6be8
--- /dev/null
+++ b/spec/services/weights_and_measures_spec.rb
@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe WeightsAndMeasures do
+  subject { WeightsAndMeasures.new(variant) }
+  let(:variant) { Spree::Variant.new }
+  let(:product) { double }
+
+  before do
+    allow(variant).to receive(:product) { product }
+  end
+
+  describe "#system" do
+    context "weight" do
+      before do
+        allow(product).to receive(:variant_unit) { "weight" }
+      end
+
+      it "when scale is for a metric unit" do
+        allow(product).to receive(:variant_unit_scale) { 1.0 }
+        expect(subject.system).to eq("metric")
+      end
+
+      it "when scale is for an imperial unit" do
+        allow(product).to receive(:variant_unit_scale) { 28.35 }
+        expect(subject.system).to eq("imperial")
+      end
+    end
+
+    context "volume" do
+      it "when scale is for a metric unit" do
+        allow(product).to receive(:variant_unit) { "volume" }
+        allow(product).to receive(:variant_unit_scale) { 1.0 }
+        expect(subject.system).to eq("metric")
+      end
+    end
+
+    context "items" do
+      it "when scale is for items" do
+        allow(product).to receive(:variant_unit) { "items" }
+        allow(product).to receive(:variant_unit_scale) { nil }
+        expect(subject.system).to eq("custom")
+      end
+    end
+  end
+
+  describe "#scale_for_unit_value" do
+    context "weight" do
+      before do
+        allow(product).to receive(:variant_unit) { "weight" }
+      end
+
+      context "metric" do
+        it "for a unit value that should display in grams" do
+          allow(product).to receive(:variant_unit_scale) { 1.0 }
+          allow(variant).to receive(:unit_value) { 500 }
+          expect(subject.scale_for_unit_value).to eq([1.0, "g"])
+        end
+
+        it "for a unit value that should display in kg" do
+          allow(product).to receive(:variant_unit_scale) { 1.0 }
+          allow(variant).to receive(:unit_value) { 1500 }
+          expect(subject.scale_for_unit_value).to eq([1000.0, "kg"])
+        end
+      end
+    end
+
+    context "volume" do
+      it "for a unit value that should display in L" do
+        allow(product).to receive(:variant_unit) { "volume" }
+        allow(product).to receive(:variant_unit_scale) { 1.0 }
+        allow(variant).to receive(:unit_value) { 1500 }
+        expect(subject.scale_for_unit_value).to eq([1000, "kL"])
+      end
+    end
+
+    context "items" do
+      it "when scale is for items" do
+        allow(product).to receive(:variant_unit) { "items" }
+        allow(product).to receive(:variant_unit_scale) { nil }
+        allow(variant).to receive(:unit_value) { 4 }
+        expect(subject.scale_for_unit_value).to eq([nil, nil])
+      end
+    end
+  end
+end