refactor: extract shared builder module, add cleaner DSL, fix type safety

Extract ConversionTableBuilderBase module to DRY up cycle detection,
graph validation, and conversion caching shared by both builders.

Add convert_to:/forward:/backward:/description: kwargs to
UnitSystemBuilder#unit as a cleaner alternative to the value: hash format.

Fix IDENTITY lambda to preserve input types, coerce convert inputs to
Rational for exact arithmetic, and use Rational(1) in comparable_amount.

Add UnitSystem#functional? for introspection.

Author: ntn

lib/measured/conversion_table_builder.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 class Measured::ConversionTableBuilder
-  attr_reader :units
+  include Measured::ConversionTableBuilderBase
 
   def initialize(units, cache: nil)
     @units = units
@@ -39,23 +39,6 @@ def generate_table
     end
   end
 
-  def validate_no_cycles
-    graph = units.select { |unit| unit.conversion_unit.present? }.group_by { |unit| unit.name }
-    validate_acyclic_graph(graph, from: graph.keys[0])
-  end
-
-  # This uses a depth-first search algorithm: https://en.wikipedia.org/wiki/Depth-first_search
-  def validate_acyclic_graph(graph, from:, visited: [])
-    graph[from]&.each do |edge|
-      adjacent_node = edge.conversion_unit
-      if visited.include?(adjacent_node)
-        raise Measured::CycleDetected.new(edge)
-      else
-        validate_acyclic_graph(graph, from: adjacent_node, visited: visited + [adjacent_node])
-      end
-    end
-  end
-
   def find_conversion(to:, from:)
     conversion = find_direct_conversion_cached(to: to, from: from) || find_tree_traversal_conversion(to: to, from: from)
 
@@ -64,17 +47,6 @@ def find_conversion(to:, from:)
     conversion
   end
 
-  def find_direct_conversion_cached(to:, from:)
-    @direct_conversion_cache ||= {}
-    @direct_conversion_cache[to] ||= {}
-
-    if @direct_conversion_cache[to].key?(from)
-       @direct_conversion_cache[to][from]
-    else
-      @direct_conversion_cache[to][from] = find_direct_conversion(to: to, from: from)
-    end
-  end
-
   def find_direct_conversion(to:, from:)
     units.each do |unit|
       return unit.conversion_amount if unit.name == from && unit.conversion_unit == to

lib/measured/conversion_table_builder_base.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+module Measured::ConversionTableBuilderBase
+  attr_reader :units
+
+  private
+
+  def validate_no_cycles
+    graph = units.select { |unit| unit.conversion_unit.present? }.group_by(&:name)
+    validate_acyclic_graph(graph, from: graph.keys[0])
+  end
+
+  # This uses a depth-first search algorithm: https://en.wikipedia.org/wiki/Depth-first_search
+  def validate_acyclic_graph(graph, from:, visited: [])
+    graph[from]&.each do |edge|
+      adjacent_node = edge.conversion_unit
+      if visited.include?(adjacent_node)
+        raise Measured::CycleDetected.new(edge)
+      else
+        validate_acyclic_graph(graph, from: adjacent_node, visited: visited + [adjacent_node])
+      end
+    end
+  end
+
+  def find_direct_conversion_cached(to:, from:)
+    @direct_conversion_cache ||= {}
+    @direct_conversion_cache[to] ||= {}
+
+    if @direct_conversion_cache[to].key?(from)
+      @direct_conversion_cache[to][from]
+    else
+      @direct_conversion_cache[to][from] = find_direct_conversion(to: to, from: from)
+    end
+  end
+end

lib/measured/unit_system_builder.rb

@@ -5,7 +5,8 @@ def initialize
     @cache = nil
   end
 
-  def unit(unit_name, aliases: [], value: nil)
+  def unit(unit_name, aliases: [], value: nil, convert_to: nil, forward: nil, backward: nil, description: nil)
+    value = build_functional_value(convert_to: convert_to, forward: forward, backward: backward, description: description) || value
     @units << build_unit(unit_name, aliases: aliases, value: value)
     nil
   end
@@ -64,6 +65,20 @@ def build_unit(name, aliases: [], value: nil)
     unit
   end
 
+  def build_functional_value(convert_to:, forward:, backward:, description:)
+    return nil unless convert_to
+
+    unless forward && backward
+      raise Measured::UnitError, "forward: and backward: are required when convert_to: is specified"
+    end
+
+    unless description
+      raise Measured::UnitError, "description: is required when convert_to: is specified"
+    end
+
+    [{ forward: forward, backward: backward, description: description }, convert_to]
+  end
+
   def check_for_duplicate_unit_names!(unit)
     names = @units.flat_map(&:names)
     if names.any? { |name| unit.names.include?(name) }

test/unit_system_builder_test.rb

@@ -136,6 +136,75 @@ class Measured::UnitSystemBuilderTest < ActiveSupport::TestCase
     assert_equal "lb", measurable.unit_system.unit_for!(:long_ton).conversion_unit
   end
 
+  test "#unit accepts convert_to:, forward:, backward:, description: kwargs for functional conversions" do
+    measurable = Measured.build do
+      unit :C
+      unit :K, convert_to: "C",
+        forward: ->(k) { k - BigDecimal("273.15") },
+        backward: ->(c) { c + BigDecimal("273.15") },
+        description: "celsius + 273.15"
+    end
+
+    assert_equal 2, measurable.unit_names.count
+    k_unit = measurable.unit_system.unit_for!(:K)
+    assert k_unit.functional?
+    assert_equal "C", k_unit.conversion_unit
+  end
+
+  test "#unit accepts description: kwarg for functional conversions" do
+    measurable = Measured.build do
+      unit :C
+      unit :K, convert_to: "C",
+        forward: ->(k) { k - BigDecimal("273.15") },
+        backward: ->(c) { c + BigDecimal("273.15") },
+        description: "celsius + 273.15"
+    end
+
+    k_unit = measurable.unit_system.unit_for!(:K)
+    assert_equal "celsius + 273.15", k_unit.description
+  end
+
+  test "#unit raises when convert_to: is given without forward: and backward:" do
+    assert_raises Measured::UnitError do
+      Measured.build do
+        unit :C
+        unit :K, convert_to: "C", forward: ->(k) { k }
+      end
+    end
+
+    assert_raises Measured::UnitError do
+      Measured.build do
+        unit :C
+        unit :K, convert_to: "C", backward: ->(c) { c }
+      end
+    end
+  end
+
+  test "#unit raises when convert_to: is given without description:" do
+    assert_raises Measured::UnitError do
+      Measured.build do
+        unit :C
+        unit :K, convert_to: "C",
+          forward: ->(k) { k - BigDecimal("273.15") },
+          backward: ->(c) { c + BigDecimal("273.15") }
+      end
+    end
+  end
+
+  test "#unit still accepts value: with hash format for functional conversions" do
+    measurable = Measured.build do
+      unit :C
+      unit :K, value: [
+        { forward: ->(k) { k - BigDecimal("273.15") }, backward: ->(c) { c + BigDecimal("273.15") } },
+        "C"
+      ]
+    end
+
+    assert_equal 2, measurable.unit_names.count
+    k_unit = measurable.unit_system.unit_for!(:K)
+    assert k_unit.functional?
+  end
+
   test "#cache sets no cache by default" do
     measurable = Measured.build do
       unit :m