diff --git a/lib/unit/class.rb b/lib/unit/class.rb index 965a708..2266c23 100644 --- a/lib/unit/class.rb +++ b/lib/unit/class.rb @@ -131,8 +131,12 @@ def eql?(other) def <=>(other) if Numeric === other - other = coerce_numeric_compatible(other) - a, b = self.normalize, other.normalize + coerced = coerce_numeric(other) + unless compatible?(coerced) + raise ArgumentError, + "comparison of #{comparison_label(self)} with #{comparison_label(other)} failed" + end + a, b = self.normalize, coerced.normalize a.value <=> b.value elsif other.respond_to?(:coerce) apply_through_coercion(other, __method__) @@ -294,6 +298,18 @@ def apply_through_coercion(obj, oper) raise TypeError, "#{obj.class} can't be coerced into #{self.class}" end + # Label an operand for a failed-comparison message. A dimensional unit keeps + # its informative #inspect (Unit("1 m")); a dimensionless unit is just a + # number — and a coerced bare numeric arrives wrapped as one — so it is + # reported by its underlying numeric class, matching how core Ruby names the + # operands of a failed comparison (and avoiding splatting an arbitrary + # #inspect into the message). + def comparison_label(operand) + return operand.inspect if Unit === operand && !operand.unit.empty? + operand = operand.value if Unit === operand + operand.class + end + def coerce_numeric_compatible(object) object = coerce_numeric(object) raise IncompatibleUnitError, "#{inspect} and #{object.inspect} are incompatible" if !compatible?(object) diff --git a/spec/unit_spec.rb b/spec/unit_spec.rb index 5cad4bd..49d6a5d 100644 --- a/spec/unit_spec.rb +++ b/spec/unit_spec.rb @@ -86,6 +86,50 @@ expect { Unit(1) < "string" }.to raise_error(ArgumentError) end + it "should raise a descriptive ArgumentError from <=> for incompatible dimensions" do + expect { Unit(1, 'm') <=> Unit(1, 's') }.to raise_error( + ArgumentError, 'comparison of Unit("1 m") with Unit("1 s") failed' + ) + expect { Unit(1, 'g') <=> Unit(1, 'm') }.to raise_error(ArgumentError) + end + + it "should show the operand's class, not a coerced unit, for non-Unit numerics" do + expect { Unit(1, 'm') > 1.2 }.to raise_error( + ArgumentError, 'comparison of Unit("1 m") with Float failed' + ) + expect { Unit(1, 'm') > 5 }.to raise_error( + ArgumentError, 'comparison of Unit("1 m") with Integer failed' + ) + end + + it "should label operands symmetrically when the unit is on the right" do + expect { 1.2 > Unit(1, 'm') }.to raise_error( + ArgumentError, 'comparison of Float with Unit("1 m") failed' + ) + expect { 5 < Unit(1, 'm') }.to raise_error( + ArgumentError, 'comparison of Integer with Unit("1 m") failed' + ) + end + + it "should still compare units with compatible dimensions" do + expect(Unit(100, 'cm') <=> Unit(1, 'm')).to eq(0) + expect(Unit(1, 'cm') <=> Unit(1, 'm')).to eq(-1) + expect(Unit(2, 'm') <=> Unit(1, 'm')).to eq(1) + end + + it "should carry the descriptive error through every comparison path" do + expect { Unit(1, 'm') > Unit(1, 's') }.to raise_error(ArgumentError, /Unit\("1 m"\).*Unit\("1 s"\)/) + expect { [Unit(1, 'm'), Unit(2, 's')].sort }.to raise_error(ArgumentError, /incompatible|failed/) + expect { Unit(1, 'm').between?(Unit(1, 's'), Unit(2, 's')) }.to raise_error(ArgumentError) + expect { Unit(1, 'm').clamp(Unit(1, 's'), Unit(2, 's')) }.to raise_error(ArgumentError) + end + + it "should sort and aggregate compatible units" do + sorted = [Unit(3, 'm'), Unit(1, 'm'), Unit(2, 'm')].sort + expect(sorted).to eq([Unit(1, 'm'), Unit(2, 'm'), Unit(3, 'm')]) + expect([Unit(50, 'cm'), Unit(1, 'm')].max).to eq(Unit(1, 'm')) + end + it "should support eql comparison" do expect(Unit(1)).to eql(Unit(1)) expect(Unit(1.0)).not_to eql(Unit(1)) @@ -255,8 +299,8 @@ expect(Unit(100, "m")).to be > Unit(0.0001, "km") end - it "should fail comparison on differing units" do - expect { Unit(1, "second") > Unit(1, "meter") }.to raise_error(Unit::IncompatibleUnitError) + it "should fail ordered comparison on differing units" do + expect { Unit(1, "second") > Unit(1, "meter") }.to raise_error(ArgumentError) end it "should keep units when the value is zero" do