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, remove dead Proc branch from
compute_inverse, and use Rational(1) in comparable_amount.

Add UnitSystem#dynamic? for introspection.

Author: ntn

lib/measured/base.rb

@@ -47,6 +47,7 @@ def method_missing(method, *args)
 require "measured/unit"
 require "measured/unit_system"
 require "measured/unit_system_builder"
+require "measured/conversion_table_builder_base"
 require "measured/conversion_table_builder"
 require "measured/dynamic_conversion_table_builder"
 require "measured/cache/null"

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/dynamic_conversion_table_builder.rb

@@ -1,8 +1,8 @@
 # frozen_string_literal: true
 class Measured::DynamicConversionTableBuilder
-  IDENTITY = ->(x) { x * Rational(1, 1) }
+  include Measured::ConversionTableBuilderBase
 
-  attr_reader :units
+  IDENTITY = ->(x) { x }
 
   def initialize(units, cache: nil)
     @units = units
@@ -14,7 +14,7 @@ def initialize(units, cache: nil)
   end
 
   def to_h
-    generate_table
+    @table ||= generate_table
   end
 
   def update_cache
@@ -43,39 +43,12 @@ def generate_table
     end
   end
 
-  def validate_no_cycles
-    graph = units.select { |unit| unit.conversion_unit.present? }.group_by(&:name)
-    validate_acyclic_graph(graph, from: graph.keys[0])
-  end
-
-  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:)
     result = find_direct_conversion_cached(to: to, from: from) || find_tree_traversal_conversion(to: to, from: from)
     raise Measured::MissingConversionPath.new(from, to) unless result
     result
   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|
       if unit.name == from && unit.conversion_unit == to

lib/measured/unit.rb

@@ -2,7 +2,7 @@
 class Measured::Unit
   include Comparable
 
-  attr_reader :name, :names, :aliases, :conversion_amount, :conversion_unit, :unit_system, :inverse_conversion_amount
+  attr_reader :name, :names, :aliases, :conversion_amount, :conversion_unit, :unit_system, :inverse_conversion_amount, :description
 
   def initialize(name, aliases: [], value: nil, unit_system: nil)
     @name = name.to_s.freeze
@@ -58,12 +58,11 @@ def <=>(other)
   private
 
   def comparable_amount(amount)
-    amount.is_a?(Proc) ? amount.call(1) : amount
+    amount.is_a?(Proc) ? amount.call(Rational(1)) : amount
   end
 
   def compute_inverse(amount)
     return nil unless amount
-    return amount if amount.is_a?(Proc) # inverse stored separately for dynamic
 
     1 / amount
   end
@@ -72,7 +71,7 @@ def build_conversion_string
     return nil unless @conversion_amount || @conversion_unit
 
     if @conversion_amount.is_a?(Proc)
-      @description # may be nil — that's fine, to_s will just show name
+      @description
     else
       "#{@conversion_amount} #{@conversion_unit}"
     end

lib/measured/unit_system.rb

@@ -10,7 +10,7 @@ def initialize(units, cache: nil)
       next unit unless conversion_unit
       if unit.dynamic?
         unit.with(value: [
-          { forward: unit.conversion_amount, backward: unit.inverse_conversion_amount },
+          { forward: unit.conversion_amount, backward: unit.inverse_conversion_amount, description: unit.description },
           conversion_unit.name
         ])
       else
@@ -53,12 +53,16 @@ def convert(value, from:, to:)
     raise Measured::UnitError, "Cannot find conversion entry from #{from} to #{to}" unless conversion
 
     if conversion.is_a?(Proc)
-      conversion.call(value).to_r
+      conversion.call(value.to_r)
     else
       value.to_r * conversion
     end
   end
 
+  def dynamic?
+    @units.any?(&:dynamic?)
+  end
+
   def update_cache
     @conversion_table_builder.update_cache
   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_dynamic_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,18 @@ def build_unit(name, aliases: [], value: nil)
     unit
   end
 
+  def build_dynamic_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
+
+    opts = { forward: forward, backward: backward }
+    opts[:description] = description if description
+    [opts, convert_to]
+  end
+
   def check_for_duplicate_unit_names!(unit)
     names = @units.flat_map(&:names)
     if names.any? { |name| unit.names.include?(name) }

test/dynamic_conversion_table_builder_test.rb

@@ -20,8 +20,11 @@ class Measured::DynamicConversionTableBuilderTest < ActiveSupport::TestCase
       Measured::Unit.new(:base),
     ]).to_h
 
-    assert table["base"]["base"].is_a?(Proc)
-    assert_equal Rational(42, 1), table["base"]["base"].call(42)
+    identity = table["base"]["base"]
+    assert identity.is_a?(Proc)
+    assert_equal 42, identity.call(42)
+    assert_equal BigDecimal("42.5"), identity.call(BigDecimal("42.5"))
+    assert_instance_of BigDecimal, identity.call(BigDecimal("42.5"))
   end
 
   test "#to_h handles static units by wrapping them as procs" do
@@ -137,6 +140,49 @@ class Measured::DynamicConversionTableBuilderTest < ActiveSupport::TestCase
     end
   end
 
+  test "#to_h computes multi-hop paths across 4+ dynamic units" do
+    table = Measured::DynamicConversionTableBuilder.new([
+      Measured::Unit.new(:A),
+      Measured::Unit.new(:B, value: [
+        { forward: ->(b) { b * 2 }, backward: ->(a) { a / 2 } },
+        "A"
+      ]),
+      Measured::Unit.new(:C, value: [
+        { forward: ->(c) { c * 3 }, backward: ->(b) { b / 3 } },
+        "B"
+      ]),
+      Measured::Unit.new(:D, value: [
+        { forward: ->(d) { d * 5 }, backward: ->(c) { c / 5 } },
+        "C"
+      ]),
+    ]).to_h
+
+    # D -> A requires 3 hops: D -> C -> B -> A
+    # D=1 -> C=5 -> B=15 -> A=30
+    assert_equal Rational(30), table["D"]["A"].call(1)
+    # A -> D is the reverse: A=30 -> B=15 -> C=5 -> D=1
+    assert_equal Rational(1), table["A"]["D"].call(30)
+
+    %w(A B C D).each do |from|
+      %w(A B C D).each do |to|
+        assert table[from][to].is_a?(Proc), "Missing table[#{from}][#{to}]"
+        result = table[to][from].call(table[from][to].call(Rational(7)))
+        assert_equal Rational(7), result, "Round-trip failed for #{from} -> #{to} -> #{from}"
+      end
+    end
+  end
+
+  test "#to_h wraps static Rational amounts as procs preserving type" do
+    table = Measured::DynamicConversionTableBuilder.new([
+      Measured::Unit.new(:m),
+      Measured::Unit.new(:cm, value: "0.01 m"),
+    ]).to_h
+
+    assert_equal Rational(100), table["m"]["cm"].call(Rational(1))
+    assert_instance_of Rational, table["m"]["cm"].call(Rational(1))
+    assert_equal Rational(50), table["m"]["cm"].call(BigDecimal("0.5"))
+  end
+
   test "#initialize with no cache argument defaults to Null and succeeds" do
     assert_nothing_raised do
       Measured::DynamicConversionTableBuilder.new([Measured::Unit.new(:base)])

test/unit_system_builder_test.rb

@@ -136,6 +136,63 @@ 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: kwargs for dynamic 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") }
+    end
+
+    assert_equal 2, measurable.unit_names.count
+    k_unit = measurable.unit_system.unit_for!(:K)
+    assert k_unit.dynamic?
+    assert_equal "C", k_unit.conversion_unit
+  end
+
+  test "#unit accepts description: kwarg for dynamic 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 still accepts value: with hash format for dynamic 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.dynamic?
+  end
+
   test "#cache sets no cache by default" do
     measurable = Measured.build do
       unit :m