ICU-21349 Improve MeasureUnit comparators

See #1433

icu4c/source/i18n/units_complexconverter.cpp

@@ -37,28 +37,18 @@ ComplexUnitsConverter::ComplexUnitsConverter(const MeasureUnitImpl &inputUnit,
         outputUnits_.emplaceBackAndCheckErrorCode(status, units_[i]->copy(status).build(status));
     }
 
-    // NOTE:
-    //  This comparator is used to sort the units in a descending order. Therefore, we return -1 if
-    //  the left is bigger than right and so on.
+    // Sorts units in descending order. Therefore, we return -1 if
+    // the left is bigger than right and so on.
     auto descendingCompareUnits = [](const void *context, const void *left, const void *right) {
         UErrorCode status = U_ZERO_ERROR;
 
         const auto *leftPointer = static_cast<const MeasureUnitImpl *const *>(left);
         const auto *rightPointer = static_cast<const MeasureUnitImpl *const *>(right);
 
-        UnitConverter fromLeftToRight(**leftPointer,                                  //
-                                      **rightPointer,                                 //
-                                      *static_cast<const ConversionRates *>(context), //
-                                      status);
-
-        double rightFromOneLeft = fromLeftToRight.convert(1.0);
-        if (std::abs(rightFromOneLeft - 1.0) < 0.0000000001) { // Equals To
-            return 0;
-        } else if (rightFromOneLeft > 1.0) { // Greater Than
-            return -1;
-        }
-
-        return 1; // Less Than
+        return -1 * UnitConverter::compareTwoUnits(**leftPointer,                                  //
+                                                   **rightPointer,                                 //
+                                                   *static_cast<const ConversionRates *>(context), //
+                                                   status);
     };
 
     uprv_sortArray(units_.getAlias(),                                                                  //

icu4c/source/i18n/units_converter.cpp

@@ -502,6 +502,52 @@ UnitConverter::UnitConverter(const MeasureUnitImpl &source, const MeasureUnitImp
                        ratesInfo, status);
 }
 
+int32_t UnitConverter::compareTwoUnits(const MeasureUnitImpl &firstUnit,
+                                       const MeasureUnitImpl &secondUnit,
+                                       const ConversionRates &ratesInfo, UErrorCode &status) {
+    if (U_FAILURE(status)) {
+        return 0;
+    }
+
+    if (firstUnit.complexity == UMeasureUnitComplexity::UMEASURE_UNIT_MIXED ||
+        secondUnit.complexity == UMeasureUnitComplexity::UMEASURE_UNIT_MIXED) {
+        status = U_INTERNAL_PROGRAM_ERROR;
+        return 0;
+    }
+
+    Convertibility unitsState = extractConvertibility(firstUnit, secondUnit, ratesInfo, status);
+    if (U_FAILURE(status)) {
+        return 0;
+    }
+
+    if (unitsState == Convertibility::UNCONVERTIBLE || unitsState == Convertibility::RECIPROCAL) {
+        status = U_INTERNAL_PROGRAM_ERROR;
+        return 0;
+    }
+
+    // Represents the conversion factor from the firstUnit to the base unit that specified in the
+    // conversion data which is considered as the root of the firstUnit and the secondUnit.
+    Factor firstUnitToBase = loadCompoundFactor(firstUnit, ratesInfo, status);
+    Factor secondUnitToBase = loadCompoundFactor(secondUnit, ratesInfo, status);
+
+    firstUnitToBase.substituteConstants();
+    secondUnitToBase.substituteConstants();
+
+    double firstUnitToBaseConversionRate = firstUnitToBase.factorNum / firstUnitToBase.factorDen;
+    double secondUnitToBaseConversionRate = secondUnitToBase.factorNum / secondUnitToBase.factorDen;
+
+    double diff = firstUnitToBaseConversionRate - secondUnitToBaseConversionRate;
+    if (diff > 0) {
+        return 1;
+    }
+
+    if (diff < 0) {
+        return -1;
+    }
+
+    return 0;
+}
+
 double UnitConverter::convert(double inputValue) const {
     double result =
         inputValue + conversionRate_.sourceOffset; // Reset the input to the target zero index.

icu4c/source/i18n/units_converter.h

@@ -143,6 +143,16 @@ class U_I18N_API UnitConverter : public UMemory {
     UnitConverter(const MeasureUnitImpl &source, const MeasureUnitImpl &target,
                   const ConversionRates &ratesInfo, UErrorCode &status);
 
+    /**
+     * Compares two single units and returns 1 if the first one is greater, -1 if the second
+     * one is greater and 0 if they are equal.
+     *
+     * NOTE:
+     *  Compares only single units that are convertible.
+     */
+    static int32_t compareTwoUnits(const MeasureUnitImpl &firstUnit, const MeasureUnitImpl &SecondUnit,
+                                   const ConversionRates &ratesInfo, UErrorCode &status);
+
     /**
      * Convert a measurement expressed in the source unit to a measurement
      * expressed in the target unit.

icu4j/main/classes/core/src/com/ibm/icu/impl/units/MeasureUnitImpl.java

@@ -754,8 +754,9 @@ public class MeasureUnitImpl {
 
         @Override
         public int compare(MeasureUnitImpl o1, MeasureUnitImpl o2) {
-            UnitConverter fromO1toO2 = new UnitConverter(o1, o2, conversionRates);
-            return fromO1toO2.convert(BigDecimal.valueOf(1)).compareTo(BigDecimal.valueOf(1));
+            BigDecimal factor1 = this.conversionRates.getFactorToBase(o1).getConversionRate();
+            BigDecimal factor2 = this.conversionRates.getFactorToBase(o2).getConversionRate();
+            return factor1.compareTo(factor2);
         }
     }