Skip to content

Commit 55c8e14

Browse files
committed
fix: address code review feedback
- Preserve description through UnitSystem unit reconstruction - Add public description reader to Unit - Memoize DynamicConversionTableBuilder#to_h - Add clarifying comment for inverse_conversion_amount ||= ordering - Add test for description preservation in UnitSystem
1 parent 6bc22d6 commit 55c8e14

4 files changed

Lines changed: 18 additions & 3 deletions

File tree

lib/measured/dynamic_conversion_table_builder.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def initialize(units, cache: nil)
1414
end
1515

1616
def to_h
17-
generate_table
17+
@table ||= generate_table
1818
end
1919

2020
def update_cache

lib/measured/unit.rb

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@
22
class Measured::Unit
33
include Comparable
44

5-
attr_reader :name, :names, :aliases, :conversion_amount, :conversion_unit, :unit_system, :inverse_conversion_amount
5+
attr_reader :name, :names, :aliases, :conversion_amount, :conversion_unit, :unit_system, :inverse_conversion_amount, :description
66

77
def initialize(name, aliases: [], value: nil, unit_system: nil)
88
@name = name.to_s.freeze
99
@aliases = aliases.map(&:to_s).map(&:freeze).freeze
1010
@names = ([@name] + @aliases).sort!.freeze
1111
@conversion_amount, @conversion_unit = parse_value(value) if value
12+
# For dynamic units, parse_dynamic_value already sets @inverse_conversion_amount
13+
# to the backward proc. The ||= ensures we don't overwrite it with compute_inverse.
1214
@inverse_conversion_amount ||= compute_inverse(@conversion_amount)
1315
@conversion_string = build_conversion_string
1416
@unit_system = unit_system

lib/measured/unit_system.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ def initialize(units, cache: nil)
1010
next unit unless conversion_unit
1111
if unit.dynamic?
1212
unit.with(value: [
13-
{ forward: unit.conversion_amount, backward: unit.inverse_conversion_amount },
13+
{ forward: unit.conversion_amount, backward: unit.inverse_conversion_amount, description: unit.description },
1414
conversion_unit.name
1515
])
1616
else

test/unit_system_test.rb

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,19 @@ class Measured::UnitSystemTest < ActiveSupport::TestCase
197197
assert_equal BigDecimal("100"), system.convert(BigDecimal("100"), from: c_unit, to: c_unit)
198198
end
199199

200+
test "dynamic system preserves description through unit reconstruction" do
201+
c = Measured::Unit.new(:C)
202+
k = Measured::Unit.new(:K, value: [
203+
{ forward: ->(k) { k - BigDecimal("273.15") }, backward: ->(c) { c + BigDecimal("273.15") }, description: "celsius + 273.15" },
204+
"C"
205+
])
206+
system = Measured::UnitSystem.new([c, k])
207+
k_unit = system.unit_for!(:K)
208+
209+
assert_equal "celsius + 273.15", k_unit.description
210+
assert_equal "K (celsius + 273.15)", k_unit.to_s
211+
end
212+
200213
test "dynamic system #cached? returns false" do
201214
c = Measured::Unit.new(:C)
202215
k = Measured::Unit.new(:K, value: [

0 commit comments

Comments
 (0)