]> granicus.if.org Git - icu/commitdiff
ICU-21349 Make appendSingleUnit behaviour in C++ comply with Java
authoryounies <younies@chromium.org>
Mon, 30 Nov 2020 18:37:37 +0000 (18:37 +0000)
committerYounies Mahmoud <younies@chromium.org>
Mon, 30 Nov 2020 21:42:17 +0000 (01:42 +0400)
See #1486

icu4c/source/i18n/measunit.cpp
icu4c/source/i18n/measunit_extra.cpp
icu4c/source/i18n/measunit_impl.h
icu4c/source/i18n/number_formatimpl.cpp
icu4c/source/i18n/number_longnames.cpp
icu4c/source/i18n/number_usageprefs.cpp
icu4c/source/i18n/units_converter.cpp
icu4c/source/test/intltest/measfmttest.cpp
icu4j/main/classes/core/src/com/ibm/icu/impl/units/MeasureUnitImpl.java

index c3e39b9c660d89ea4ac61ff280e46d8d18240405..5e55fde4ca17a2af91c36c96525e9e329fc78de2 100644 (file)
@@ -2321,8 +2321,8 @@ MeasureUnitImpl MeasureUnitImpl::copy(UErrorCode &status) const {
     MeasureUnitImpl result;
     result.complexity = complexity;
     result.identifier.append(identifier, status);
-    for (int32_t i = 0; i < units.length(); i++) {
-        SingleUnitImpl *item = result.units.emplaceBack(*units[i]);
+    for (int32_t i = 0; i < singleUnits.length(); i++) {
+        SingleUnitImpl *item = result.singleUnits.emplaceBack(*singleUnits[i]);
         if (!item) {
             status = U_MEMORY_ALLOCATION_ERROR;
             return result;
index 2eb3f06614296767a644dfdf5a17f81d3fdf9f07..1edc77212faa438a8e7f7b381d185c5243f1bb17 100644 (file)
@@ -380,7 +380,53 @@ public:
 
     MeasureUnitImpl parse(UErrorCode& status) {
         MeasureUnitImpl result;
-        parseImpl(result, status);
+
+        if (U_FAILURE(status)) {
+            return result;
+        }
+        if (fSource.empty()) {
+            // The dimenionless unit: nothing to parse. leave result as is.
+            return result;
+        }
+
+        while (hasNext()) {
+            bool sawAnd = false;
+
+            SingleUnitImpl singleUnit = nextSingleUnit(sawAnd, status);
+            if (U_FAILURE(status)) {
+                return result;
+            }
+
+            bool added = result.appendSingleUnit(singleUnit, status);
+            if (U_FAILURE(status)) {
+                return result;
+            }
+
+            if (sawAnd && !added) {
+                // Two similar units are not allowed in a mixed unit.
+                status = kUnitIdentifierSyntaxError;
+                return result;
+            }
+
+            if (result.singleUnits.length() >= 2) {
+                // nextSingleUnit fails appropriately for "per" and "and" in the
+                // same identifier. It doesn't fail for other compound units
+                // (COMPOUND_PART_TIMES). Consequently we take care of that
+                // here.
+                UMeasureUnitComplexity complexity =
+                    sawAnd ? UMEASURE_UNIT_MIXED : UMEASURE_UNIT_COMPOUND;
+                if (result.singleUnits.length() == 2) {
+                    // After appending two singleUnits, the complexity will be `UMEASURE_UNIT_COMPOUND`
+                    U_ASSERT(result.complexity == UMEASURE_UNIT_COMPOUND);
+                    result.complexity = complexity;
+                } else if (result.complexity != complexity) {
+                    // Can't have mixed compound units
+                    status = kUnitIdentifierSyntaxError;
+                    return result;
+                }
+            }
+        }
+
         return result;
     }
 
@@ -457,9 +503,10 @@ private:
      * unit", sawAnd is set to true. If not, it is left as is.
      * @param status ICU error code.
      */
-    void nextSingleUnit(SingleUnitImpl& result, bool& sawAnd, UErrorCode& status) {
+    SingleUnitImpl nextSingleUnit(bool &sawAnd, UErrorCode &status) {
+        SingleUnitImpl result;
         if (U_FAILURE(status)) {
-            return;
+            return result;
         }
 
         // state:
@@ -470,7 +517,9 @@ private:
 
         bool atStart = fIndex == 0;
         Token token = nextToken(status);
-        if (U_FAILURE(status)) { return; }
+        if (U_FAILURE(status)) {
+            return result;
+        }
 
         if (atStart) {
             // Identifiers optionally start with "per-".
@@ -480,14 +529,16 @@ private:
                 result.dimensionality = -1;
 
                 token = nextToken(status);
-                if (U_FAILURE(status)) { return; }
+                if (U_FAILURE(status)) {
+                    return result;
+                }
             }
         } else {
             // All other SingleUnit's are separated from previous SingleUnit's
             // via a compound part:
             if (token.getType() != Token::TYPE_COMPOUND_PART) {
                 status = kUnitIdentifierSyntaxError;
-                return;
+                return result;
             }
 
             switch (token.getMatch()) {
@@ -496,7 +547,7 @@ private:
                     // Mixed compound units not yet supported,
                     // TODO(CLDR-13700).
                     status = kUnitIdentifierSyntaxError;
-                    return;
+                    return result;
                 }
                 fAfterPer = true;
                 result.dimensionality = -1;
@@ -513,14 +564,16 @@ private:
                     // Can't start with "-and-", and mixed compound units
                     // not yet supported, TODO(CLDR-13700).
                     status = kUnitIdentifierSyntaxError;
-                    return;
+                    return result;
                 }
                 sawAnd = true;
                 break;
             }
 
             token = nextToken(status);
-            if (U_FAILURE(status)) { return; }
+            if (U_FAILURE(status)) {
+                return result;
+            }
         }
 
         // Read tokens until we have a complete SingleUnit or we reach the end.
@@ -529,7 +582,7 @@ private:
                 case Token::TYPE_POWER_PART:
                     if (state > 0) {
                         status = kUnitIdentifierSyntaxError;
-                        return;
+                        return result;
                     }
                     result.dimensionality *= token.getPower();
                     state = 1;
@@ -538,7 +591,7 @@ private:
                 case Token::TYPE_SI_PREFIX:
                     if (state > 1) {
                         status = kUnitIdentifierSyntaxError;
-                        return;
+                        return result;
                     }
                     result.siPrefix = token.getSIPrefix();
                     state = 2;
@@ -546,67 +599,25 @@ private:
 
                 case Token::TYPE_SIMPLE_UNIT:
                     result.index = token.getSimpleUnitIndex();
-                    return;
+                    return result;
 
                 default:
                     status = kUnitIdentifierSyntaxError;
-                    return;
+                    return result;
             }
 
             if (!hasNext()) {
                 // We ran out of tokens before finding a complete single unit.
                 status = kUnitIdentifierSyntaxError;
-                return;
+                return result;
             }
             token = nextToken(status);
             if (U_FAILURE(status)) {
-                return;
+                return result;
             }
         }
-    }
 
-    /// @param result is modified, not overridden. Caller must pass in a
-    /// default-constructed (empty) MeasureUnitImpl instance.
-    void parseImpl(MeasureUnitImpl& result, UErrorCode& status) {
-        if (U_FAILURE(status)) {
-            return;
-        }
-        if (fSource.empty()) {
-            // The dimenionless unit: nothing to parse. leave result as is.
-            return;
-        }
-        int32_t unitNum = 0;
-        while (hasNext()) {
-            bool sawAnd = false;
-            SingleUnitImpl singleUnit;
-            nextSingleUnit(singleUnit, sawAnd, status);
-            if (U_FAILURE(status)) {
-                return;
-            }
-            U_ASSERT(!singleUnit.isDimensionless());
-            bool added = result.append(singleUnit, status);
-            if (sawAnd && !added) {
-                // Two similar units are not allowed in a mixed unit
-                status = kUnitIdentifierSyntaxError;
-                return;
-            }
-            if ((++unitNum) >= 2) {
-                // nextSingleUnit fails appropriately for "per" and "and" in the
-                // same identifier. It doesn't fail for other compound units
-                // (COMPOUND_PART_TIMES). Consequently we take care of that
-                // here.
-                UMeasureUnitComplexity complexity =
-                    sawAnd ? UMEASURE_UNIT_MIXED : UMEASURE_UNIT_COMPOUND;
-                if (unitNum == 2) {
-                    U_ASSERT(result.complexity == UMEASURE_UNIT_SINGLE);
-                    result.complexity = complexity;
-                } else if (result.complexity != complexity) {
-                    // Can't have mixed compound units
-                    status = kUnitIdentifierSyntaxError;
-                    return;
-                }
-            }
-        }
+        return result;
     }
 };
 
@@ -684,32 +695,26 @@ void serialize(MeasureUnitImpl& impl, UErrorCode& status) {
         return;
     }
     U_ASSERT(impl.identifier.isEmpty());
-    if (impl.units.length() == 0) {
+    if (impl.singleUnits.length() == 0) {
         // Dimensionless, constructed by the default constructor: no appending
         // to impl.identifier, we wish it to contain the zero-length string.
         return;
     }
     if (impl.complexity == UMEASURE_UNIT_COMPOUND) {
         // Note: don't sort a MIXED unit
-        uprv_sortArray(
-            impl.units.getAlias(),
-            impl.units.length(),
-            sizeof(impl.units[0]),
-            compareSingleUnits,
-            nullptr,
-            false,
-            &status);
+        uprv_sortArray(impl.singleUnits.getAlias(), impl.singleUnits.length(),
+                       sizeof(impl.singleUnits[0]), compareSingleUnits, nullptr, false, &status);
         if (U_FAILURE(status)) {
             return;
         }
     }
-    serializeSingle(*impl.units[0], true, impl.identifier, status);
-    if (impl.units.length() == 1) {
+    serializeSingle(*impl.singleUnits[0], true, impl.identifier, status);
+    if (impl.singleUnits.length() == 1) {
         return;
     }
-    for (int32_t i = 1; i < impl.units.length(); i++) {
-        const SingleUnitImpl& prev = *impl.units[i-1];
-        const SingleUnitImpl& curr = *impl.units[i];
+    for (int32_t i = 1; i < impl.singleUnits.length(); i++) {
+        const SingleUnitImpl &prev = *impl.singleUnits[i - 1];
+        const SingleUnitImpl &curr = *impl.singleUnits[i];
         if (impl.complexity == UMEASURE_UNIT_MIXED) {
             impl.identifier.append("-and-", status);
             serializeSingle(curr, true, impl.identifier, status);
@@ -722,41 +727,6 @@ void serialize(MeasureUnitImpl& impl, UErrorCode& status) {
             serializeSingle(curr, false, impl.identifier, status);
         }
     }
-
-}
-
-/**
- * Appends a SingleUnitImpl to a MeasureUnitImpl.
- *
- * @return true if a new item was added. If unit is the dimensionless unit, it
- * is never added: the return value will always be false.
- */
-bool appendImpl(MeasureUnitImpl& impl, const SingleUnitImpl& unit, UErrorCode& status) {
-    if (unit.isDimensionless()) {
-        // We don't append dimensionless units.
-        return false;
-    }
-    // Find a similar unit that already exists, to attempt to coalesce
-    SingleUnitImpl* oldUnit = nullptr;
-    for (int32_t i = 0; i < impl.units.length(); i++) {
-        auto* candidate = impl.units[i];
-        if (candidate->isCompatibleWith(unit)) {
-            oldUnit = candidate;
-        }
-    }
-    if (oldUnit) {
-        // Both dimensionalities will be positive, or both will be negative, by
-        // virtue of isCompatibleWith().
-        oldUnit->dimensionality += unit.dimensionality;
-    } else {
-        SingleUnitImpl* destination = impl.units.emplaceBack();
-        if (!destination) {
-            status = U_MEMORY_ALLOCATION_ERROR;
-            return false;
-        }
-        *destination = unit;
-    }
-    return (oldUnit == nullptr);
 }
 
 } // namespace
@@ -768,11 +738,11 @@ SingleUnitImpl SingleUnitImpl::forMeasureUnit(const MeasureUnit& measureUnit, UE
     if (U_FAILURE(status)) {
         return {};
     }
-    if (impl.units.length() == 0) {
+    if (impl.singleUnits.length() == 0) {
         return {};
     }
-    if (impl.units.length() == 1) {
-        return *impl.units[0];
+    if (impl.singleUnits.length() == 1) {
+        return *impl.singleUnits[0];
     }
     status = U_ILLEGAL_ARGUMENT_ERROR;
     return {};
@@ -780,7 +750,7 @@ SingleUnitImpl SingleUnitImpl::forMeasureUnit(const MeasureUnit& measureUnit, UE
 
 MeasureUnit SingleUnitImpl::build(UErrorCode& status) const {
     MeasureUnitImpl temp;
-    temp.append(*this, status);
+    temp.appendSingleUnit(*this, status);
     return std::move(temp).build(status);
 }
 
@@ -793,7 +763,7 @@ MeasureUnitImpl::MeasureUnitImpl(const MeasureUnitImpl &other, UErrorCode &statu
 }
 
 MeasureUnitImpl::MeasureUnitImpl(const SingleUnitImpl &singleUnit, UErrorCode &status) {
-    this->append(singleUnit, status);
+    this->appendSingleUnit(singleUnit, status);
 }
 
 MeasureUnitImpl MeasureUnitImpl::forIdentifier(StringPiece identifier, UErrorCode& status) {
@@ -821,14 +791,51 @@ MeasureUnitImpl MeasureUnitImpl::forMeasureUnitMaybeCopy(
 
 void MeasureUnitImpl::takeReciprocal(UErrorCode& /*status*/) {
     identifier.clear();
-    for (int32_t i = 0; i < units.length(); i++) {
-        units[i]->dimensionality *= -1;
+    for (int32_t i = 0; i < singleUnits.length(); i++) {
+        singleUnits[i]->dimensionality *= -1;
     }
 }
 
-bool MeasureUnitImpl::append(const SingleUnitImpl& singleUnit, UErrorCode& status) {
+bool MeasureUnitImpl::appendSingleUnit(const SingleUnitImpl &singleUnit, UErrorCode &status) {
     identifier.clear();
-    return appendImpl(*this, singleUnit, status);
+
+    if (singleUnit.isDimensionless()) {
+        // Do not append dimensionless units.
+        return false;
+    }
+
+    // Find a similar unit that already exists, to attempt to coalesce
+    SingleUnitImpl *oldUnit = nullptr;
+    for (int32_t i = 0; i < this->singleUnits.length(); i++) {
+        auto *candidate = this->singleUnits[i];
+        if (candidate->isCompatibleWith(singleUnit)) {
+            oldUnit = candidate;
+        }
+    }
+
+    if (oldUnit) {
+        // Both dimensionalities will be positive, or both will be negative, by
+        // virtue of isCompatibleWith().
+        oldUnit->dimensionality += singleUnit.dimensionality;
+
+        return false;
+    }
+
+    // Add a copy of singleUnit
+    // NOTE: MaybeStackVector::emplaceBackAndCheckErrorCode creates new copy of  singleUnit.
+    this->singleUnits.emplaceBackAndCheckErrorCode(status, singleUnit);
+    if (U_FAILURE(status)) {
+        return false;
+    }
+
+    // If the MeasureUnitImpl is `UMEASURE_UNIT_SINGLE` and after the appending a unit, the `singleUnits`
+    // contains more than one. thus means the complexity should be `UMEASURE_UNIT_COMPOUND`
+    if (this->singleUnits.length() > 1 &&
+        this->complexity == UMeasureUnitComplexity::UMEASURE_UNIT_SINGLE) {
+        this->complexity = UMeasureUnitComplexity::UMEASURE_UNIT_COMPOUND;
+    }
+
+    return true;
 }
 
 MaybeStackVector<MeasureUnitImpl> MeasureUnitImpl::extractIndividualUnits(UErrorCode &status) const {
@@ -839,8 +846,8 @@ MaybeStackVector<MeasureUnitImpl> MeasureUnitImpl::extractIndividualUnits(UError
         return result;
     }
 
-    for (int32_t i = 0; i < units.length(); i++) {
-        result.emplaceBackAndCheckErrorCode(status, *units[i], status);
+    for (int32_t i = 0; i < singleUnits.length(); i++) {
+        result.emplaceBackAndCheckErrorCode(status, *singleUnits[i], status);
     }
 
     return result;
@@ -899,10 +906,10 @@ MeasureUnit MeasureUnit::product(const MeasureUnit& other, UErrorCode& status) c
         status = U_ILLEGAL_ARGUMENT_ERROR;
         return {};
     }
-    for (int32_t i = 0; i < otherImpl.units.length(); i++) {
-        impl.append(*otherImpl.units[i], status);
+    for (int32_t i = 0; i < otherImpl.singleUnits.length(); i++) {
+        impl.appendSingleUnit(*otherImpl.singleUnits[i], status);
     }
-    if (impl.units.length() > 1) {
+    if (impl.singleUnits.length() > 1) {
         impl.complexity = UMEASURE_UNIT_COMPOUND;
     }
     return std::move(impl).build(status);
@@ -911,14 +918,14 @@ MeasureUnit MeasureUnit::product(const MeasureUnit& other, UErrorCode& status) c
 LocalArray<MeasureUnit> MeasureUnit::splitToSingleUnitsImpl(int32_t& outCount, UErrorCode& status) const {
     MeasureUnitImpl temp;
     const MeasureUnitImpl& impl = MeasureUnitImpl::forMeasureUnit(*this, temp, status);
-    outCount = impl.units.length();
+    outCount = impl.singleUnits.length();
     MeasureUnit* arr = new MeasureUnit[outCount];
     if (arr == nullptr) {
         status = U_MEMORY_ALLOCATION_ERROR;
         return LocalArray<MeasureUnit>();
     }
     for (int32_t i = 0; i < outCount; i++) {
-        arr[i] = impl.units[i]->build(status);
+        arr[i] = impl.singleUnits[i]->build(status);
     }
     return LocalArray<MeasureUnit>(arr, status);
 }
index 3cc2cd0476f75a83da451f9469c021696bf3ff40..a4f97cffe731f4d54927edaef537565d68c1e7be 100644 (file)
@@ -215,19 +215,19 @@ struct U_I18N_API MeasureUnitImpl : public UMemory {
      * @return true if a new item was added. If unit is the dimensionless unit,
      * it is never added: the return value will always be false.
      */
-    bool append(const SingleUnitImpl& singleUnit, UErrorCode& status);
+    bool appendSingleUnit(const SingleUnitImpl& singleUnit, UErrorCode& status);
 
     /** The complexity, either SINGLE, COMPOUND, or MIXED. */
     UMeasureUnitComplexity complexity = UMEASURE_UNIT_SINGLE;
 
     /**
-     * The list of simple units. These may be summed or multiplied, based on the
+     * The list of single units. These may be summed or multiplied, based on the
      * value of the complexity field.
      *
      * The "dimensionless" unit (SingleUnitImpl default constructor) must not be
      * added to this list.
      */
-    MaybeStackVector<SingleUnitImpl> units;
+    MaybeStackVector<SingleUnitImpl> singleUnits;
 
     /**
      * The full unit identifier.  Owned by the MeasureUnitImpl.  Empty if not computed.
index f7e1fcbb4e6c0b7e71c20759a14eea48d636da17..140913147692ccf73ca7855398aa313fd333cc90 100644 (file)
@@ -252,8 +252,8 @@ NumberFormatterImpl::macrosToMicroGenerator(const MacroProps& macros, bool safe,
     } else if (isMixedUnit) {
         MeasureUnitImpl temp;
         const MeasureUnitImpl &outputUnit = MeasureUnitImpl::forMeasureUnit(macros.unit, temp, status);
-        auto unitConversionHandler =
-            new UnitConversionHandler(outputUnit.units[0]->build(status), macros.unit, chain, status);
+        auto unitConversionHandler = new UnitConversionHandler(outputUnit.singleUnits[0]->build(status),
+                                                               macros.unit, chain, status);
         fUnitConversionHandler.adoptInsteadAndCheckErrorCode(unitConversionHandler, status);
         chain = fUnitConversionHandler.getAlias();
     }
index 0fa56c70c1569581f0280bdda388821b6156df1b..bf89a7a0b9e748b207e9a8357038170d9e60c921 100644 (file)
@@ -234,13 +234,13 @@ void LongNameHandler::forMeasureUnit(const Locale &loc, const MeasureUnit &unitR
         MeasureUnitImpl fullUnit = MeasureUnitImpl::forMeasureUnitMaybeCopy(unitRef, status);
         MeasureUnitImpl unit;
         MeasureUnitImpl perUnit;
-        for (int32_t i = 0; i < fullUnit.units.length(); i++) {
-            SingleUnitImpl *subUnit = fullUnit.units[i];
+        for (int32_t i = 0; i < fullUnit.singleUnits.length(); i++) {
+            SingleUnitImpl *subUnit = fullUnit.singleUnits[i];
             if (subUnit->dimensionality > 0) {
-                unit.append(*subUnit, status);
+                unit.appendSingleUnit(*subUnit, status);
             } else {
                 subUnit->dimensionality *= -1;
-                perUnit.append(*subUnit, status);
+                perUnit.appendSingleUnit(*subUnit, status);
             }
         }
         forCompoundUnit(loc, std::move(unit).build(status), std::move(perUnit).build(status), width,
@@ -420,12 +420,12 @@ void MixedUnitLongNameHandler::forMeasureUnit(const Locale &loc, const MeasureUn
 
     MeasureUnitImpl temp;
     const MeasureUnitImpl& impl = MeasureUnitImpl::forMeasureUnit(mixedUnit, temp, status);
-    fillIn->fMixedUnitCount = impl.units.length();
+    fillIn->fMixedUnitCount = impl.singleUnits.length();
     fillIn->fMixedUnitData.adoptInstead(new UnicodeString[fillIn->fMixedUnitCount * ARRAY_LENGTH]);
     for (int32_t i = 0; i < fillIn->fMixedUnitCount; i++) {
         // Grab data for each of the components.
         UnicodeString *unitData = &fillIn->fMixedUnitData[i * ARRAY_LENGTH];
-        getMeasureData(loc, impl.units[i]->build(status), width, unitData, status);
+        getMeasureData(loc, impl.singleUnits[i]->build(status), width, unitData, status);
     }
 
     UListFormatterWidth listWidth = ULISTFMT_WIDTH_SHORT;
index 0d9cb06c50a2ab25e52922be96380f942f5f6120..ea684bad870ce58bed9e89522e48b26bf35cb6e1 100644 (file)
@@ -116,9 +116,9 @@ void mixedMeasuresToMicros(const MaybeStackVector<Measure> &measures, DecimalQua
         MeasureUnitImpl temp;
         const MeasureUnitImpl& impl = MeasureUnitImpl::forMeasureUnit(micros->outputUnit, temp, status);
         U_ASSERT(U_SUCCESS(status));
-        U_ASSERT(measures.length() == impl.units.length());
+        U_ASSERT(measures.length() == impl.singleUnits.length());
         for (int32_t i = 0; i < measures.length(); i++) {
-            U_ASSERT(measures[i]->getUnit() == impl.units[i]->build(status));
+            U_ASSERT(measures[i]->getUnit() == impl.singleUnits[i]->build(status));
         }
         (void)impl;
 #endif
index bed40dd4e13ab82a43796964ec71f86ee43769b5..085789abc3042167fb90eeec8f63d334a82e8fee 100644 (file)
@@ -217,8 +217,8 @@ Factor loadCompoundFactor(const MeasureUnitImpl &source, const ConversionRates &
                           UErrorCode &status) {
 
     Factor result;
-    for (int32_t i = 0, n = source.units.length(); i < n; i++) {
-        SingleUnitImpl singleUnit = *source.units[i];
+    for (int32_t i = 0, n = source.singleUnits.length(); i < n; i++) {
+        SingleUnitImpl singleUnit = *source.singleUnits[i];
 
         Factor singleFactor = loadSingleFactor(singleUnit.getSimpleUnitID(), ratesInfo, status);
         if (U_FAILURE(status)) return result;
@@ -248,12 +248,12 @@ UBool checkSimpleUnit(const MeasureUnitImpl &unit, UErrorCode &status) {
     if (unit.complexity != UMEASURE_UNIT_SINGLE) {
         return false;
     }
-    if (unit.units.length() == 0) {
+    if (unit.singleUnits.length() == 0) {
         // Empty units means simple unit.
         return true;
     }
 
-    auto singleUnit = *(unit.units[0]);
+    auto singleUnit = *(unit.singleUnits[0]);
 
     if (singleUnit.dimensionality != 1 || singleUnit.siPrefix != UMEASURE_SI_PREFIX_ONE) {
         return false;
@@ -329,8 +329,8 @@ void mergeSingleUnitWithDimension(MaybeStackVector<UnitIndexAndDimension> &unitI
 
 void mergeUnitsAndDimensions(MaybeStackVector<UnitIndexAndDimension> &unitIndicesWithDimension,
                              const MeasureUnitImpl &shouldBeMerged, int32_t multiplier) {
-    for (int32_t unit_i = 0; unit_i < shouldBeMerged.units.length(); unit_i++) {
-        auto singleUnit = *shouldBeMerged.units[unit_i];
+    for (int32_t unit_i = 0; unit_i < shouldBeMerged.singleUnits.length(); unit_i++) {
+        auto singleUnit = *shouldBeMerged.singleUnits[unit_i];
         mergeSingleUnitWithDimension(unitIndicesWithDimension, singleUnit, multiplier);
     }
 }
@@ -396,7 +396,7 @@ MeasureUnitImpl U_I18N_API extractCompoundBaseUnit(const MeasureUnitImpl &source
     MeasureUnitImpl result;
     if (U_FAILURE(status)) return result;
 
-    const auto &singleUnits = source.units;
+    const auto &singleUnits = source.singleUnits;
     for (int i = 0, count = singleUnits.length(); i < count; ++i) {
         const auto &singleUnit = *singleUnits[i];
         // Extract `ConversionRateInfo` using the absolute unit. For example: in case of `square-meter`,
@@ -414,11 +414,11 @@ MeasureUnitImpl U_I18N_API extractCompoundBaseUnit(const MeasureUnitImpl &source
         // Multiply the power of the singleUnit by the power of the baseUnit. For example, square-hectare
         // must be pow4-meter. (NOTE: hectare --> square-meter)
         auto baseUnits =
-            MeasureUnitImpl::forIdentifier(rateInfo->baseUnit.toStringPiece(), status).units;
+            MeasureUnitImpl::forIdentifier(rateInfo->baseUnit.toStringPiece(), status).singleUnits;
         for (int32_t i = 0, baseUnitsCount = baseUnits.length(); i < baseUnitsCount; i++) {
             baseUnits[i]->dimensionality *= singleUnit.dimensionality;
             // TODO: Deal with SI-prefix
-            result.append(*baseUnits[i], status);
+            result.appendSingleUnit(*baseUnits[i], status);
 
             if (U_FAILURE(status)) {
                 return result;
index 7d5330f66571d66a38bb1874b2418fec31996d28..3f95358fb0072fed81601a7b3889a635c32a31fd 100644 (file)
@@ -4046,8 +4046,8 @@ void MeasureFormatTest::TestInternalMeasureUnitImpl() {
     status.assertSuccess();
     assertEquals("mu1 initial identifier", "", mu1.identifier.data());
     assertEquals("mu1 initial complexity", UMEASURE_UNIT_SINGLE, mu1.complexity);
-    assertEquals("mu1 initial units length", 1, mu1.units.length());
-    assertEquals("mu1 initial units[0]", "meter", mu1.units[0]->getSimpleUnitID());
+    assertEquals("mu1 initial units length", 1, mu1.singleUnits.length());
+    assertEquals("mu1 initial units[0]", "meter", mu1.singleUnits[0]->getSimpleUnitID());
 
     // Producing identifier via build(): the std::move() means mu1 gets modified
     // while it also gets assigned to tmp's internal fImpl.
@@ -4055,8 +4055,8 @@ void MeasureFormatTest::TestInternalMeasureUnitImpl() {
     status.assertSuccess();
     assertEquals("mu1 post-move-build identifier", "meter", mu1.identifier.data());
     assertEquals("mu1 post-move-build complexity", UMEASURE_UNIT_SINGLE, mu1.complexity);
-    assertEquals("mu1 post-move-build units length", 1, mu1.units.length());
-    assertEquals("mu1 post-move-build units[0]", "meter", mu1.units[0]->getSimpleUnitID());
+    assertEquals("mu1 post-move-build units length", 1, mu1.singleUnits.length());
+    assertEquals("mu1 post-move-build units[0]", "meter", mu1.singleUnits[0]->getSimpleUnitID());
     assertEquals("MeasureUnit tmp identifier", "meter", tmp.getIdentifier());
 
     // This temporary variable is used when forMeasureUnit's first parameter
@@ -4066,8 +4066,8 @@ void MeasureFormatTest::TestInternalMeasureUnitImpl() {
     status.assertSuccess();
     assertEquals("tmpMemory identifier", "", tmpMemory.identifier.data());
     assertEquals("tmpMemory complexity", UMEASURE_UNIT_SINGLE, tmpMemory.complexity);
-    assertEquals("tmpMemory units length", 1, tmpMemory.units.length());
-    assertEquals("tmpMemory units[0]", "meter", tmpMemory.units[0]->getSimpleUnitID());
+    assertEquals("tmpMemory units length", 1, tmpMemory.singleUnits.length());
+    assertEquals("tmpMemory units[0]", "meter", tmpMemory.singleUnits[0]->getSimpleUnitID());
     assertEquals("tmpImplRef identifier", "", tmpImplRef.identifier.data());
     assertEquals("tmpImplRef complexity", UMEASURE_UNIT_SINGLE, tmpImplRef.complexity);
 
@@ -4076,18 +4076,39 @@ void MeasureFormatTest::TestInternalMeasureUnitImpl() {
     mu1 = std::move(mu2);
     assertEquals("mu1 = move(mu2): identifier", "", mu1.identifier.data());
     assertEquals("mu1 = move(mu2): complexity", UMEASURE_UNIT_COMPOUND, mu1.complexity);
-    assertEquals("mu1 = move(mu2): units length", 2, mu1.units.length());
-    assertEquals("mu1 = move(mu2): units[0]", "newton", mu1.units[0]->getSimpleUnitID());
-    assertEquals("mu1 = move(mu2): units[1]", "meter", mu1.units[1]->getSimpleUnitID());
+    assertEquals("mu1 = move(mu2): units length", 2, mu1.singleUnits.length());
+    assertEquals("mu1 = move(mu2): units[0]", "newton", mu1.singleUnits[0]->getSimpleUnitID());
+    assertEquals("mu1 = move(mu2): units[1]", "meter", mu1.singleUnits[1]->getSimpleUnitID());
 
     mu1 = MeasureUnitImpl::forIdentifier("hour-and-minute-and-second", status);
     status.assertSuccess();
     assertEquals("mu1 = HMS: identifier", "", mu1.identifier.data());
     assertEquals("mu1 = HMS: complexity", UMEASURE_UNIT_MIXED, mu1.complexity);
-    assertEquals("mu1 = HMS: units length", 3, mu1.units.length());
-    assertEquals("mu1 = HMS: units[0]", "hour", mu1.units[0]->getSimpleUnitID());
-    assertEquals("mu1 = HMS: units[1]", "minute", mu1.units[1]->getSimpleUnitID());
-    assertEquals("mu1 = HMS: units[2]", "second", mu1.units[2]->getSimpleUnitID());
+    assertEquals("mu1 = HMS: units length", 3, mu1.singleUnits.length());
+    assertEquals("mu1 = HMS: units[0]", "hour", mu1.singleUnits[0]->getSimpleUnitID());
+    assertEquals("mu1 = HMS: units[1]", "minute", mu1.singleUnits[1]->getSimpleUnitID());
+    assertEquals("mu1 = HMS: units[2]", "second", mu1.singleUnits[2]->getSimpleUnitID());
+
+    MeasureUnitImpl m2 = MeasureUnitImpl::forIdentifier("", status);
+    m2.appendSingleUnit(SingleUnitImpl::forMeasureUnit(MeasureUnit::getMeter(), status), status);
+    m2.appendSingleUnit(SingleUnitImpl::forMeasureUnit(MeasureUnit::getMeter(), status), status);
+    status.assertSuccess();
+    assertEquals("append meter twice: complexity", UMEASURE_UNIT_SINGLE, m2.complexity);
+    assertEquals("append meter twice: units length", 1, m2.singleUnits.length());
+    assertEquals("append meter twice: units[0]", "meter", m2.singleUnits[0]->getSimpleUnitID());
+    assertEquals("append meter twice: identifier", "square-meter",
+                 std::move(m2).build(status).getIdentifier());
+
+    MeasureUnitImpl mcm = MeasureUnitImpl::forIdentifier("", status);
+    mcm.appendSingleUnit(SingleUnitImpl::forMeasureUnit(MeasureUnit::getMeter(), status), status);
+    mcm.appendSingleUnit(SingleUnitImpl::forMeasureUnit(MeasureUnit::getCentimeter(), status), status);
+    status.assertSuccess();
+    assertEquals("append meter & centimeter: complexity", UMEASURE_UNIT_COMPOUND, mcm.complexity);
+    assertEquals("append meter & centimeter: units length", 2, mcm.singleUnits.length());
+    assertEquals("append meter & centimeter: units[0]", "meter", mcm.singleUnits[0]->getSimpleUnitID());
+    assertEquals("append meter & centimeter: units[1]", "meter", mcm.singleUnits[1]->getSimpleUnitID());
+    assertEquals("append meter & centimeter: identifier", "centimeter-meter",
+                 std::move(mcm).build(status).getIdentifier());
 }
 
 
index cdaca18febd5528856e81a7c86e5f7a26ab2698c..0821aa24588f803f84eee51a32df85386dd485f2 100644 (file)
@@ -27,7 +27,7 @@ public class MeasureUnitImpl {
     private MeasureUnit.Complexity complexity = MeasureUnit.Complexity.SINGLE;
 
     /**
-     * The list of simple units. These may be summed or multiplied, based on the
+     * The list of single units. These may be summed or multiplied, based on the
      * value of the complexity field.
      * <p>
      * The "dimensionless" unit (SingleUnitImpl default constructor) must not be
@@ -141,7 +141,7 @@ public class MeasureUnitImpl {
         identifier = null;
 
         if (singleUnit == null) {
-            // We don't append dimensionless units.
+            // Do not append dimensionless units.
             return false;
         }
 
@@ -165,8 +165,8 @@ public class MeasureUnitImpl {
         // Add a copy of singleUnit
         this.singleUnits.add(singleUnit.copy());
 
-        // If the MeasureUnitImpl is `UMEASURE_UNIT_SINGLE` and after the appending a unit, the singleUnits are more
-        // than one singleUnit. thus means the complexity should be `UMEASURE_UNIT_COMPOUND`
+        // If the MeasureUnitImpl is `UMEASURE_UNIT_SINGLE` and after the appending a unit, the singleUnits contains
+        // more than one. thus means the complexity should be `UMEASURE_UNIT_COMPOUND`
         if (this.singleUnits.size() > 1 && this.complexity == MeasureUnit.Complexity.SINGLE) {
             this.setComplexity(MeasureUnit.Complexity.COMPOUND);
         }
@@ -495,7 +495,7 @@ public class MeasureUnitImpl {
                     MeasureUnit.Complexity complexity =
                             fSawAnd ? MeasureUnit.Complexity.MIXED : MeasureUnit.Complexity.COMPOUND;
                     if (result.getSingleUnits().size() == 2) {
-                        // After appending two singleUnits, the complexity will be `UMEASURE_UNIT_COMPOUND`
+                        // After appending two singleUnits, the complexity will be MeasureUnit.Complexity.COMPOUND
                         assert result.getComplexity() == MeasureUnit.Complexity.COMPOUND;
                         result.setComplexity(complexity);
                     } else if (result.getComplexity() != complexity) {