finalize Units Router

diff --git a/icu4c/source/i18n/complexunitsconverter.cpp b/icu4c/source/i18n/complexunitsconverter.cpp
index da4c0686588d4125defad5cedb2782863adaaaa0..7ec36b23fd197ea1bf8c6c2c63e8b6f404b80852 100644 (file)
--- a/icu4c/source/i18n/complexunitsconverter.cpp
+++ b/icu4c/source/i18n/complexunitsconverter.cpp
@@ -28,7 +28,7 @@ ComplexUnitsConverter::ComplexUnitsConverter(const MeasureUnit inputUnit, const
     // In case the `outputUnits` are `UMEASURE_UNIT_MIXED` such as `foot+inch`. In this case we need more
     // converters to convert from the `inputUnit` to the first unit in the `outputUnits`. Then, a
     // converter from the first unit in the `outputUnits` to the second unit and so on.
-    //      For Example: 
+    //      For Example:
     //          - inputUnit is `meter`
     //          - outputUnits is `foot+inch`
     //              - Therefore, we need to have two converters:
@@ -36,8 +36,9 @@ ComplexUnitsConverter::ComplexUnitsConverter(const MeasureUnit inputUnit, const
     //                      2. a converter from `foot` to `inch`
     //          - Therefore, if the input is `2 meter`:
     //              1. convert `meter` to `foot` --> 2 meter to 6.56168 feet
-    //              2. convert the residual of 6.56168 feet (0.56168) to inches, which will be (6.74016 inches)
-    //              3. then, the final result will be (6 feet and 6.74016 inches)     
+    //              2. convert the residual of 6.56168 feet (0.56168) to inches, which will be (6.74016
+    //              inches)
+    //              3. then, the final result will be (6 feet and 6.74016 inches)
     int32_t length;
     auto singleUnits = outputUnits.splitToSingleUnits(length, status);
     MaybeStackVector<MeasureUnit> singleUnitsInOrder;
@@ -59,17 +60,16 @@ ComplexUnitsConverter::ComplexUnitsConverter(const MeasureUnit inputUnit, const
                                         status);
         }
 
-        if (U_FAILURE(status)) return;
+        if (U_FAILURE(status)) { return; }
     }
 
     units_.appendAll(singleUnitsInOrder, status);
 }
 
 UBool ComplexUnitsConverter::greaterThanOrEqual(double quantity, double limit) const {
-    // TODO(younies): this assert fails for the first constructor above:
     U_ASSERT(unitConverters_.length() > 0);
 
-    // first quantity is the biggest one.
+    // First converter converts to the biggest quantity.
     double newQuantity = unitConverters_[0]->convert(quantity);
 
     return newQuantity >= limit;
@@ -80,21 +80,21 @@ MaybeStackVector<Measure> ComplexUnitsConverter::convert(double quantity, UError
 
     for (int i = 0, n = unitConverters_.length(); i < n; ++i) {
         quantity = (*unitConverters_[i]).convert(quantity);
-        if (i < n - 1) { // not last element
+        if (i < n - 1) {
             int64_t newQuantity = quantity;
             Formattable formattableNewQuantity(newQuantity);
-            // Measure wants to own its MeasureUnit. For now, this copies it.
-            // TODO(younies): consider whether ownership transfer would be
-            // reasonable? (If not, just delete this comment?)
-            result.emplaceBack(formattableNewQuantity, new MeasureUnit(*units_[i]), status);
 
+            // NOTE: Measure would own its MeasureUnit.
+            result.emplaceBack(formattableNewQuantity, std::move(MeasureUnit(*units_[i])), status);
+
+            // Keep the residual of the quantity.
+            //   For example: `3.6 feet`, keep only `0.6 feet`
             quantity -= newQuantity;
-        } else { // Last element
+        } else { // LAST ELEMENT
             Formattable formattableQuantity(quantity);
-            // Measure wants to own its MeasureUnit. For now, this copies it.
-            //  TODO(younies): consider whether ownership transfer would be
-            //  reasonable? (If not, just delete this comment?)
-            result.emplaceBack(formattableQuantity, new MeasureUnit(*units_[i]), status);
+
+            // NOTE: Measure would own its MeasureUnit.
+            result.emplaceBack(formattableQuantity, std::move(MeasureUnit(*units_[i])), status);
         }
     }
 

diff --git a/icu4c/source/i18n/complexunitsconverter.h b/icu4c/source/i18n/complexunitsconverter.h
index eaef67fc8a232d38257db7c153a1925ca2b8f0bd..9dce941fe59b5a6fc9988106a3b185de4a737cf3 100644 (file)
--- a/icu4c/source/i18n/complexunitsconverter.h
+++ b/icu4c/source/i18n/complexunitsconverter.h
@@ -18,8 +18,7 @@ U_NAMESPACE_BEGIN
 
 /**
  *  Convert from single unit to multiple/complex unit. For example, from `meter` to `foot+inch`.
- *  
- *  
+ *
  *  DESIGN:
  *    This class uses `UnitConverter` in order to perform the single converter (i.e. converters from a
  *    single unit to another single unit). Therefore, `ComplexUnitsConverter` class contains multiple
@@ -51,7 +50,7 @@ class U_I18N_API ComplexUnitsConverter {
     //    - E.g. converting meters to feet and inches.
     //                  1 meter --> 3 feet, 3.3701 inches
     //         NOTE:
-    //           the smallest element is the only element that has fractional values.
+    //           the smallest element is the only element that could has fractional values.
     MaybeStackVector<Measure> convert(double quantity, UErrorCode &status) const;
 
   private:

diff --git a/icu4c/source/i18n/unitsrouter.cpp b/icu4c/source/i18n/unitsrouter.cpp
index 16c52763916475f9f1fcf39fc7038e999dfbade1..cf90044453bce950df5a9ae6918792bec6f57ec4 100644 (file)
--- a/icu4c/source/i18n/unitsrouter.cpp
+++ b/icu4c/source/i18n/unitsrouter.cpp
@@ -48,13 +48,7 @@ UnitsRouter::UnitsRouter(MeasureUnit inputUnit, StringPiece region, StringPiece
 
         converterPreferences_.emplaceBack(inputUnit, complexTargetUnit, preference.geq, conversionRates,
                                           status);
-        if (U_FAILURE(status)) {
-            fprintf(
-                stderr,
-                "FAILED: converterPreferences_.emplaceBack(<%s>, <%s>, %f, conversionRates, status)\n",
-                inputUnit.getIdentifier(), complexTargetUnit.getIdentifier(), preference.geq);
-            return;
-        }
+        if (U_FAILURE(status)) { return; }
     }
 }
 
@@ -63,9 +57,9 @@ MaybeStackVector<Measure> UnitsRouter::route(double quantity, UErrorCode &status
 
         const auto &converterPreference = *converterPreferences_[i];
 
-        if (i == n - 1) { // Last element
-            return converterPreference.converter.convert(quantity, status);
-        }
+        // In case of the last converter, the conversion will performed even the value is less than the
+        // limit.
+        if (i == n - 1) { return converterPreference.converter.convert(quantity, status); }
 
         if (converterPreference.converter.greaterThanOrEqual(quantity, converterPreference.limit)) {
             return converterPreference.converter.convert(quantity, status);