Merge pull request #59 from andi-huber/master

Solving unit equivalence
unitsofmeasurement · Apr 22, 2018 · 5274b0a · 5274b0a
2 parents fc3a3eb + 6bc1c5b
commit 5274b0a
Show file tree

Hide file tree

Showing 15 changed files with 705 additions and 133 deletions.
diff --git a/src/main/java/tech/units/indriya/AbstractConverter.java b/src/main/java/tech/units/indriya/AbstractConverter.java
@@ -72,7 +72,7 @@ public abstract class AbstractConverter
 	/**
 	 * Holds identity converter.
 	 */
-	@Deprecated //[ahuber] potentially misused: checking whether a UnitConverter is an identity operator
+	// [ahuber] potentially misused: checking whether a UnitConverter is an identity operator
 	// should be done with unitConverter.isIdentity() rather then unitConverter == AbstractConverter.IDENTITY
 	public static final AbstractConverter IDENTITY = new Identity();
 
@@ -83,37 +83,32 @@ protected AbstractConverter() {
 	}
 
 	/**
-	 * Concatenates this physics converter with another physics converter. The
-	 * resulting converter is equivalent to first converting by the specified
-	 * converter (right converter), and then converting by this converter (left
-	 * converter).
-	 *
+	 * Guard for {@link #simpleCompose(AbstractConverter)}
 	 * @param that
-	 *            the other converter.
-	 * @return the concatenation of this converter with that converter.
+	 * @return whether or not a 'simple' composition of transformations is possible
 	 */
-	public AbstractConverter concatenate(AbstractConverter that) {
-		return (that == IDENTITY) ? this : new Pair(this, that);
+	protected abstract boolean isSimpleCompositionWith(AbstractConverter that);
+
+	/**
+	 * Guarded by {@link #isSimpleCompositionWith(AbstractConverter)}
+	 * @param that
+	 * @return a new AbstractConverter that adds no additional conversion step
+	 */
+	protected AbstractConverter simpleCompose(AbstractConverter that) {
+		throw new IllegalStateException(
+				String.format("Concrete UnitConverter '%s' does not implement simpleCompose(...).", this)); 
 	}
 
 	/**
 	 * Creates a converter with the specified Prefix.
 	 * 
 	 * @param prefix
 	 *            the prefix for the factor.
-	 * @throws IllegalArgumentException
-	 *             if coefficient is <code>1.0</code> (would result in identity
-	 *             converter)
 	 */
 	public static UnitConverter of(Prefix prefix) {
 		return PowerConverter.of(prefix);
 	}
 
-	@Override
-	public boolean isIdentity() {
-		return false;
-	}
-
 	@Override
 	public abstract boolean equals(Object cvtr);
 
@@ -124,8 +119,21 @@ public boolean isIdentity() {
 	public abstract AbstractConverter inverse();
 
 	@Override
-	public UnitConverter concatenate(UnitConverter converter) {
-		return (converter == IDENTITY) ? this : new Pair(this, converter);
+	public final UnitConverter concatenate(UnitConverter converter) {
+		Objects.requireNonNull(converter, "Can not concatenate null.");
+		if(converter instanceof AbstractConverter) {
+			// let Simplifier decide
+			AbstractConverter other = (AbstractConverter) converter;
+			return AbstractUnit.Simplifier.compose(this, other);
+		}
+		// converter is not known to this implementation ...
+		if(converter.isIdentity()) {
+			return this;
+		}
+		if(this.isIdentity()) {
+			return converter;
+		}
+		return new Pair(this, converter);
 	}
 
 	@Override
@@ -139,18 +147,20 @@ public List<? extends UnitConverter> getConversionSteps() {
 	 * @throws IllegalArgumentException
 	 *             if the value is </code>null</code>.
 	 */
-	public Number convert(Number value) {
+	public final Number convert(Number value) {
+		if(isIdentity()) {
+			return value;
+		}
+		if (value == null) {
+			throw new IllegalArgumentException("Value cannot be null");
+		}
 		if (value instanceof BigDecimal) {
 			return convert((BigDecimal) value, MathContext.DECIMAL128);
 		}
 		if (value instanceof BigInteger) {
 			return convert((BigInteger) value, MathContext.DECIMAL128);
 		}
-		if (value != null) {
-			return convert(value.doubleValue());
-		} else {
-			throw new IllegalArgumentException("Value cannot be null");
-		}
+		return convert(value.doubleValue());
 	}
 
 	@Override
@@ -197,11 +207,6 @@ public BigDecimal convert(BigDecimal value, MathContext ctx) {
 			return value;
 		}
 
-		@Override
-		public UnitConverter concatenate(UnitConverter converter) {
-			return converter;
-		}
-
 		@Override
 		public boolean equals(Object cvtr) {
 			return (cvtr instanceof Identity);
@@ -224,6 +229,21 @@ public int compareTo(UnitConverter o) {
 			}
 			return -1;
 		}
+
+		@Override
+		protected boolean isSimpleCompositionWith(AbstractConverter that) {
+			return true; // composition with identity can always be simplified
+		}
+
+		@Override
+		protected AbstractConverter simpleCompose(AbstractConverter that) {
+			if(that.isIdentity()) {
+				// if both are identities, let the default implementation take precedence
+				return this; 
+			}
+			return that;
+		}
+
 	}
 
 	/**
@@ -360,6 +380,13 @@ public String toString() {
 					.map(UnitConverter::toString)
 					.collect(Collectors.joining(", ")) );
 		}
+
+		@Override
+		protected boolean isSimpleCompositionWith(AbstractConverter that) {
+			return false;
+		}
 
 	}
+
+
 }
diff --git a/src/main/java/tech/units/indriya/AbstractUnit.java b/src/main/java/tech/units/indriya/AbstractUnit.java
@@ -344,7 +344,7 @@ public final Unit<Q> transform(UnitConverter operation) {
 		} else {
 			cvtr = operation;
 		}
-		if (cvtr.equals(AbstractConverter.IDENTITY)) {
+		if (cvtr.isIdentity()) {
 			return systemUnit;
 		} else {
 			return new TransformedUnit<>(null, this, systemUnit, cvtr);
@@ -593,7 +593,8 @@ public int compareTo(Unit<Q> that) {
 	public boolean isEquivalentOf(Unit<Q> that) {
 		if (this.compareTo(that) == 0)
 			return true;
-		return this.getConverterTo(that).equals(that.getConverterTo(this));
+		return this.getConverterTo(that).isIdentity();
+		//[ahuber] was ... return this.getConverterTo(that).equals(that.getConverterTo(this));
 	}
 
 	// //////////////////////////////////////////////////////////////
@@ -634,6 +635,47 @@ public static boolean areEqual(@SuppressWarnings("rawtypes") AbstractUnit u1, @S
 		}
 	}
 
+	/**
+	 * Utility class for UnitConverter composition and equivalence decision
+	 */
+	public static final class Simplifier {
+
+		public static AbstractConverter compose(AbstractConverter a, AbstractConverter b) {
+
+			if(a.isIdentity()) {
+				if(b.isIdentity()) {
+					return normalFormOrder(a, b)<=0 ? a : b;
+				}
+				return b;
+			}
+			if(b.isIdentity()) {
+				return a;
+			}
+
+			if(a.isSimpleCompositionWith(b)) {
+				return a.simpleCompose(b);
+			}
+
+			// TODO simplify if a or b are instances of Pair, will require 'commutes' checks
+
+			return normalFormOrder(a, b)<=0 ? new AbstractConverter.Pair(a, b) : new AbstractConverter.Pair(b, a);
+		}
+
+		private static int normalFormOrder(AbstractConverter a, AbstractConverter b) {
+
+			if(a.getClass().equals(b.getClass())) {
+
+				if(a instanceof PowerConverter) {
+					return Integer.compare( ((PowerConverter)a).getBase(), ((PowerConverter)b).getBase() );
+				}
+
+				return 0;
+			}
+			// TODO lookup a table ?
+			return 0;
+		}
+	}
+
 
 
 }
diff --git a/src/main/java/tech/units/indriya/function/AddConverter.java b/src/main/java/tech/units/indriya/function/AddConverter.java
@@ -29,7 +29,6 @@
  */
 package tech.units.indriya.function;
 
-import java.io.Serializable;
 import java.math.BigDecimal;
 import java.math.MathContext;
 import java.util.Objects;
@@ -57,19 +56,15 @@ public final class AddConverter extends AbstractConverter implements ValueSuppli
   /**
    * Holds the offset.
    */
-  private double offset;
+  private final double offset;
 
   /**
    * Creates an additive converter having the specified offset.
    *
    * @param offset
    *          the offset value.
-   * @throws IllegalArgumentException
-   *           if offset is <code>0.0</code> (would result in identity converter).
    */
   public AddConverter(double offset) {
-    if (offset == 0.0)
-      throw new IllegalArgumentException("Would result in identity converter");
     this.offset = offset;
   }
 
@@ -81,15 +76,22 @@ public AddConverter(double offset) {
   public double getOffset() {
     return offset;
   }
+
+  @Override
+  public boolean isIdentity() {
+    return offset == 0.0;
+  }
 
   @Override
-  public UnitConverter concatenate(UnitConverter converter) {
-    if (!(converter instanceof AddConverter))
-      return super.concatenate(converter);
-    double newOffset = offset + ((AddConverter) converter).offset;
-    return newOffset == 0.0 ? IDENTITY : new AddConverter(newOffset);
+  protected boolean isSimpleCompositionWith(AbstractConverter that) {
+  	return that instanceof AddConverter;
   }
 
+  @Override
+  protected AbstractConverter simpleCompose(AbstractConverter that) {
+    return new AddConverter(offset + ((AddConverter)that).offset);
+  }
+
   @Override
   public AddConverter inverse() {
     return new AddConverter(-offset);
@@ -130,7 +132,7 @@ public int hashCode() {
 
   @Override
   public boolean isLinear() {
-    return false;
+    return isIdentity();
   }
 
   public Double getValue() {
@@ -147,4 +149,6 @@ public int compareTo(UnitConverter o) {
     }
     return -1;
   }
+
+
 }
diff --git a/src/main/java/tech/units/indriya/function/ExpConverter.java b/src/main/java/tech/units/indriya/function/ExpConverter.java
@@ -98,6 +98,24 @@ public double getBase() {
     return base;
   }
 
+  @Override
+  public boolean isIdentity() {
+    return false;
+  }
+
+  @Override
+  protected boolean isSimpleCompositionWith(AbstractConverter that) {
+	if(that instanceof LogConverter) {
+		return ((LogConverter)that).getBase() == base; // can compose with log to identity, provided it has same base
+	}
+  	return false;
+  }
+
+  @Override
+  protected AbstractConverter simpleCompose(AbstractConverter that) {
+    return AbstractConverter.IDENTITY;
+  }
+
   @Override
   public AbstractConverter inverse() {
     return new LogConverter(base);
@@ -162,4 +180,6 @@ public int compareTo(UnitConverter o) {
     }
     return -1;
   }
+
+
 }
diff --git a/src/main/java/tech/units/indriya/function/LogConverter.java b/src/main/java/tech/units/indriya/function/LogConverter.java
@@ -85,6 +85,24 @@ public LogConverter(double base) {
   public double getBase() {
     return base;
   }
+
+  @Override
+  public boolean isIdentity() {
+    return false;
+  }
+
+  @Override
+  protected boolean isSimpleCompositionWith(AbstractConverter that) {
+	if(that instanceof ExpConverter) {
+		return ((ExpConverter)that).getBase() == base; // can compose with exp to identity, provided it has same base
+	}
+  	return false;
+  }
+
+  @Override
+  protected AbstractConverter simpleCompose(AbstractConverter that) {
+    return AbstractConverter.IDENTITY;
+  }
 
   @Override
   public AbstractConverter inverse() {