Skip to content

Commit

Permalink
Merge pull request #297 from andi-huber/master
Browse files Browse the repository at this point in the history 
294: fixes 'Multiplication should carry over operand Units if possible'
  • Loading branch information
@andi-huber
andi-huber authored Jul 21, 2020
2 parents dfe2bf7 + 0482b17 commit 0ffb7f0
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 66 deletions.
140 changes: 83 additions & 57 deletions src/main/java/tech/units/indriya/internal/function/ScaleHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down Expand Up @@ -130,16 +111,19 @@ public static <Q extends Quantity<Q>> ComparableQuantity<Q> scalarMultiplication
final Quantity<Q> quantity,
final UnaryOperator<Number> 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)
Expand All @@ -153,43 +137,47 @@ public static ComparableQuantity<?> multiplication(
final Quantity<?> q2,
final BinaryOperator<Number> amountOperator,
final BinaryOperator<Unit<?>> 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 <Q extends Quantity<Q>> UnsupportedOperationException unsupportedRelativeScaleConversion(
Quantity<Q> quantity,
Unit<Q> anotherUnit) {
return new UnsupportedOperationException(
String.format(
"Conversion of Quantitity %s to Unit %s is not supported for realtive scale.",
quantity, anotherUnit));
// -- HELPER

private static <Q extends Quantity<Q>> Quantity<Q> toAbsoluteLinear(Quantity<Q> quantity) {
final Unit<Q> 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 <Q extends Quantity<Q>> ToSystemUnitConverter toSystemUnitConverterForAdd(
final Quantity<Q> q1,
final Quantity<Q> q2) {
final Unit<Q> systemUnit = q1.getUnit().getSystemUnit();
return ToSystemUnitConverter.forQuantity(q2, systemUnit);
}

// used for multiplication, also honors RELATIVE scale
// used for multiplication, honors RELATIVE scale
private static <T extends Quantity<T>>
ToSystemUnitConverter toSystemUnitConverterForMul(Quantity<T> quantity) {
final Unit<T> systemUnit = quantity.getUnit().getSystemUnit();
Expand All @@ -202,7 +190,7 @@ private static Optional<Number> linearFactorOf(UnitConverter converter) {
: Optional.empty();
}

// also honors RELATIVE scale
// honors RELATIVE scale
private static class ToSystemUnitConverter implements UnaryOperator<Number> {
private final UnaryOperator<Number> unaryOperator;
private final UnaryOperator<Number> inverseOperator;
Expand Down Expand Up @@ -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<Number> unaryOperator, UnaryOperator<Number> inverseOperator) {
private ToSystemUnitConverter(
UnaryOperator<Number> unaryOperator,
UnaryOperator<Number> inverseOperator) {
this.unaryOperator = unaryOperator;
this.inverseOperator = inverseOperator;
}
Expand All @@ -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 <Q extends Quantity<Q>> UnsupportedOperationException unsupportedRelativeScaleConversion(
Quantity<Q> quantity,
Unit<Q> 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));
}

}
2 changes: 1 addition & 1 deletion src/test/java/tech/units/indriya/quantity/DoubleComparableQuantityTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public void multiplyTest() {
assertEquals(result.getUnit(), Units.METRE);
@SuppressWarnings("unchecked")
ComparableQuantity<Length> result2 = (ComparableQuantity<Length>) metre.multiply(Quantities.getQuantity(10D, Units.HOUR));
assertEquals("360000 m·s", result2.toString());
assertEquals("100 m·h", result2.toString());
}

@Test
Expand Down
1 change: 0 additions & 1 deletion src/test/java/tech/units/indriya/quantity/NumberComparableQuantityTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
11 changes: 4 additions & 7 deletions src/test/java/tech/units/indriya/quantity/RelativeQuantitiesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -88,7 +87,6 @@ void relativeTemperatureRoundTrip() {
}

//[indriya#294]
@Disabled //FIXME
@Test @DisplayName("Multiplication should carry over operand Units (abs. scale)")
void intuitivelyKeepUnits_whenAbsolute() {

Expand All @@ -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() {

Expand All @@ -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());
}


Expand Down

0 comments on commit 0ffb7f0

Please sign in to comment.