From 0482b174f29f1f964613f0b19d4b58316e1b48a0 Mon Sep 17 00:00:00 2001 From: Andi Huber Date: Tue, 21 Jul 2020 14:02:18 +0200 Subject: [PATCH] 294: fixes 'Multiplication should carry over operand Units if possible' --- .../internal/function/ScaleHelper.java | 140 +++++++++++------- .../DoubleComparableQuantityTest.java | 2 +- .../NumberComparableQuantityTest.java | 1 - .../quantity/RelativeQuantitiesTest.java | 11 +- 4 files changed, 88 insertions(+), 66 deletions(-) diff --git a/src/main/java/tech/units/indriya/internal/function/ScaleHelper.java b/src/main/java/tech/units/indriya/internal/function/ScaleHelper.java index b4023b1d..5efc5d00 100644 --- a/src/main/java/tech/units/indriya/internal/function/ScaleHelper.java +++ b/src/main/java/tech/units/indriya/internal/function/ScaleHelper.java @@ -52,25 +52,6 @@ */ public final class ScaleHelper { - public static enum OperandMode { - ALL_ABSOLUTE, - ALL_RELATIVE, - MIXED; - public static OperandMode get( - final Quantity q1, - final Quantity q2) { - if(q1.getScale()!=q2.getScale()) { - return OperandMode.MIXED; - } - return isAbsolute(q1) - ? OperandMode.ALL_ABSOLUTE - : OperandMode.ALL_RELATIVE; - } - public boolean isAllRelative() { - return this==ALL_RELATIVE; - } - } - public static boolean isAbsolute(final Quantity quantity) { return ABSOLUTE==quantity.getScale(); } @@ -130,16 +111,19 @@ public static > ComparableQuantity scalarMultiplication final Quantity quantity, final UnaryOperator operator) { - // if operands has scale RELATIVE, multiplication is trivial + // if operand has scale RELATIVE, multiplication is trivial if (isRelative(quantity)) { - return Quantities.getQuantity(operator.apply(quantity.getValue()), quantity.getUnit(), RELATIVE); + return Quantities.getQuantity( + operator.apply(quantity.getValue()), + quantity.getUnit(), + RELATIVE); } - final ToSystemUnitConverter thisConverter = toSystemUnitConverterForMul(quantity); + final ToSystemUnitConverter toSystemUnits = toSystemUnitConverterForMul(quantity); - final Number thisValueWithAbsoluteScale = thisConverter.apply(quantity.getValue()); + final Number thisValueWithAbsoluteScale = toSystemUnits.apply(quantity.getValue()); final Number resultValueInAbsUnits = operator.apply(thisValueWithAbsoluteScale); - final boolean needsInvering = !thisConverter.isNoop(); + final boolean needsInvering = !toSystemUnits.isNoop(); final Number resultValueInThisUnit = needsInvering ? quantity.getUnit().getConverterTo(quantity.getUnit().getSystemUnit()).inverse().convert(resultValueInAbsUnits) @@ -153,35 +137,39 @@ public static ComparableQuantity multiplication( final Quantity q2, final BinaryOperator amountOperator, final BinaryOperator> unitOperator) { - - final ToSystemUnitConverter thisConverter = toSystemUnitConverterForMul(q1); - final ToSystemUnitConverter thatConverter = toSystemUnitConverterForMul(q2); - - final Number thisValueWithAbsoluteScale = thisConverter.apply(q1.getValue()); - final Number thatValueWithAbsoluteScale = thatConverter.apply(q2.getValue()); - - final Number resultValueInSystemUnits = amountOperator.apply(thisValueWithAbsoluteScale, thatValueWithAbsoluteScale); - - final Unit thisAbsUnit = thisConverter.isNoop() ? q1.getUnit() : q1.getUnit().getSystemUnit(); - final Unit thatAbsUnit = thatConverter.isNoop() ? q2.getUnit() : q2.getUnit().getSystemUnit(); - + + final Quantity absQ1 = toAbsoluteLinear(q1); + final Quantity absQ2 = toAbsoluteLinear(q2); return Quantities.getQuantity( - resultValueInSystemUnits, - unitOperator.apply(thisAbsUnit, thatAbsUnit)); + amountOperator.apply(absQ1.getValue(), absQ2.getValue()), + unitOperator.apply(absQ1.getUnit(), absQ2.getUnit())); } - private static > UnsupportedOperationException unsupportedRelativeScaleConversion( - Quantity quantity, - Unit anotherUnit) { - return new UnsupportedOperationException( - String.format( - "Conversion of Quantitity %s to Unit %s is not supported for realtive scale.", - quantity, anotherUnit)); + // -- HELPER + + private static > Quantity toAbsoluteLinear(Quantity quantity) { + final Unit systemUnit = quantity.getUnit().getSystemUnit(); + final UnitConverter toSystemUnit = quantity.getUnit().getConverterTo(systemUnit); + if(toSystemUnit.isLinear()) { + if(isAbsolute(quantity)) { + return quantity; + } + return Quantities.getQuantity(quantity.getValue(), quantity.getUnit()); + } + // convert to system units + if(isAbsolute(quantity)) { + return Quantities.getQuantity(toSystemUnit.convert(quantity.getValue()), systemUnit, Scale.ABSOLUTE); + } else { + final Number linearFactor = linearFactorOf(toSystemUnit).orElse(null); + if(linearFactor==null) { + throw unsupportedRelativeScaleConversion(quantity, systemUnit); + } + final Number valueInSystemUnits = Calculator.of(linearFactor).multiply(quantity.getValue()).peek(); + return Quantities.getQuantity(valueInSystemUnits, systemUnit, Scale.ABSOLUTE); + } } - - // -- HELPER - LOW LEVEL - // used for addition, also honors RELATIVE scale + // used for addition, honors RELATIVE scale private static > ToSystemUnitConverter toSystemUnitConverterForAdd( final Quantity q1, final Quantity q2) { @@ -189,7 +177,7 @@ private static > ToSystemUnitConverter toSystemUnitConvert return ToSystemUnitConverter.forQuantity(q2, systemUnit); } - // used for multiplication, also honors RELATIVE scale + // used for multiplication, honors RELATIVE scale private static > ToSystemUnitConverter toSystemUnitConverterForMul(Quantity quantity) { final Unit systemUnit = quantity.getUnit().getSystemUnit(); @@ -202,7 +190,7 @@ private static Optional linearFactorOf(UnitConverter converter) { : Optional.empty(); } - // also honors RELATIVE scale + // honors RELATIVE scale private static class ToSystemUnitConverter implements UnaryOperator { private final UnaryOperator unaryOperator; private final UnaryOperator inverseOperator; @@ -247,7 +235,9 @@ public static ToSystemUnitConverter factor(Number factor) { number->Calculator.of(number).multiply(factor).peek(), number->Calculator.of(number).divide(factor).peek()); } - private ToSystemUnitConverter(UnaryOperator unaryOperator, UnaryOperator inverseOperator) { + private ToSystemUnitConverter( + UnaryOperator unaryOperator, + UnaryOperator inverseOperator) { this.unaryOperator = unaryOperator; this.inverseOperator = inverseOperator; } @@ -261,13 +251,49 @@ public Number apply(Number x) { : unaryOperator.apply(x); } - private static UnsupportedOperationException unsupportedConverter(UnitConverter converter, Unit unit) { - return new UnsupportedOperationException( - String.format( - "Scale conversion from RELATIVE to ABSOLUTE for Unit %s having Converter %s is not implemented.", - unit, converter)); + } + + // -- OPERANDS + + private static enum OperandMode { + ALL_ABSOLUTE, + ALL_RELATIVE, + MIXED; + public static OperandMode get( + final Quantity q1, + final Quantity q2) { + if(q1.getScale()!=q2.getScale()) { + return OperandMode.MIXED; + } + return isAbsolute(q1) + ? OperandMode.ALL_ABSOLUTE + : OperandMode.ALL_RELATIVE; } - +// public boolean isAllAbsolute() { +// return this==ALL_ABSOLUTE; +// } + public boolean isAllRelative() { + return this==ALL_RELATIVE; + } + } + + + // -- EXCEPTIONS + + private static > UnsupportedOperationException unsupportedRelativeScaleConversion( + Quantity quantity, + Unit anotherUnit) { + return new UnsupportedOperationException( + String.format( + "Conversion of Quantitity %s to Unit %s is not supported for realtive scale.", + quantity, anotherUnit)); + } + + private static UnsupportedOperationException unsupportedConverter(UnitConverter converter, Unit unit) { + return new UnsupportedOperationException( + String.format( + "Scale conversion from RELATIVE to ABSOLUTE for Unit %s having Converter %s is not implemented.", + unit, converter)); } } diff --git a/src/test/java/tech/units/indriya/quantity/DoubleComparableQuantityTest.java b/src/test/java/tech/units/indriya/quantity/DoubleComparableQuantityTest.java index bf3d5cfe..3c28b74d 100644 --- a/src/test/java/tech/units/indriya/quantity/DoubleComparableQuantityTest.java +++ b/src/test/java/tech/units/indriya/quantity/DoubleComparableQuantityTest.java @@ -147,7 +147,7 @@ public void multiplyTest() { assertEquals(result.getUnit(), Units.METRE); @SuppressWarnings("unchecked") ComparableQuantity result2 = (ComparableQuantity) metre.multiply(Quantities.getQuantity(10D, Units.HOUR)); - assertEquals("360000 m·s", result2.toString()); + assertEquals("100 m·h", result2.toString()); } @Test diff --git a/src/test/java/tech/units/indriya/quantity/NumberComparableQuantityTest.java b/src/test/java/tech/units/indriya/quantity/NumberComparableQuantityTest.java index 9faf0408..52fc4c30 100644 --- a/src/test/java/tech/units/indriya/quantity/NumberComparableQuantityTest.java +++ b/src/test/java/tech/units/indriya/quantity/NumberComparableQuantityTest.java @@ -44,7 +44,6 @@ import org.junit.jupiter.api.Test; import tech.units.indriya.ComparableQuantity; -import tech.units.indriya.quantity.Quantities; import tech.units.indriya.unit.Units; public class NumberComparableQuantityTest { diff --git a/src/test/java/tech/units/indriya/quantity/RelativeQuantitiesTest.java b/src/test/java/tech/units/indriya/quantity/RelativeQuantitiesTest.java index 846c4bd7..ef201851 100644 --- a/src/test/java/tech/units/indriya/quantity/RelativeQuantitiesTest.java +++ b/src/test/java/tech/units/indriya/quantity/RelativeQuantitiesTest.java @@ -30,8 +30,8 @@ package tech.units.indriya.quantity; import javax.measure.Quantity; -import javax.measure.Unit; import javax.measure.Quantity.Scale; +import javax.measure.Unit; import javax.measure.quantity.Energy; import javax.measure.quantity.Temperature; import javax.measure.quantity.Volume; @@ -40,7 +40,6 @@ import static javax.measure.MetricPrefix.MILLI; import org.junit.jupiter.api.BeforeAll; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -88,7 +87,6 @@ void relativeTemperatureRoundTrip() { } //[indriya#294] - @Disabled //FIXME @Test @DisplayName("Multiplication should carry over operand Units (abs. scale)") void intuitivelyKeepUnits_whenAbsolute() { @@ -100,12 +98,11 @@ void intuitivelyKeepUnits_whenAbsolute() { assertEquals("2 cal/ml", energyPerVolume.multiply(2).toString()); // when multiply by 'ml', keep 'cal' as unit (don't convert to system units) - assertEquals("1 ml", energyPerVolume.multiply(Quantities.getQuantity(1, UNIT_MILLILITRE)).toString()); + assertEquals("1 cal", energyPerVolume.multiply(Quantities.getQuantity(1, UNIT_MILLILITRE)).toString()); } //[indriya#294] - @Disabled //FIXME @Test @DisplayName("Multiplication should carry over operand Units if possible") void intuitivelyKeepUnits_whenRelative() { @@ -116,8 +113,8 @@ void intuitivelyKeepUnits_whenRelative() { // when scalar multiply, keep units (don't convert to system units) assertEquals("20 ℃", deltaT.multiply(2).toString()); - // convert Celsius[℃] to Kelvin[k], but keep Hour[h] (don't convert to system unit Second[s]) - assertEquals("20 K/h", temperaturePerTime.toString()); + // convert Celsius[℃] to Kelvin[K], but keep Hour[h] (don't convert to system unit Second[s]) + assertEquals("10 K/h", temperaturePerTime.toString()); }