]> granicus.if.org Git - icu/commitdiff
Use 'baseUnit' for conversion pivot unit rather than 'target'.
authorHugo van der Merwe <17109322+hugovdm@users.noreply.github.com>
Wed, 25 Mar 2020 18:25:33 +0000 (19:25 +0100)
committerHugo van der Merwe <17109322+hugovdm@users.noreply.github.com>
Wed, 25 Mar 2020 18:25:33 +0000 (19:25 +0100)
icu4c/source/i18n/getunitsdata.cpp
icu4c/source/i18n/getunitsdata.h
icu4c/source/i18n/unitconverter.cpp
icu4c/source/test/intltest/unitstest.cpp

index 7217f02d49fd1d1f680374f5db24421f04a63e17..16041ba0df89cac0f2be4e28292d3d74d03b1241 100644 (file)
@@ -50,8 +50,8 @@ class ConversionRateDataSink : public ResourceSink {
      * constructor, but only if an identical instance isn't already present.
      *
      * @param source The source unit identifier.
-     * @param value A resource containing conversion rate info (a target and
-     * factor, and possibly an offset).
+     * @param value A resource containing conversion rate info (the base unit
+     * and factor, and possibly an offset).
      * @param noFallback Ignored.
      * @param status The standard ICU error code output parameter.
      */
@@ -59,21 +59,21 @@ class ConversionRateDataSink : public ResourceSink {
         ResourceTable conversionRateTable = value.getTable(status);
         if (U_FAILURE(status)) return;
 
-        // Collect target, factor and offset from the resource.
+        // Collect base unit, factor and offset from the resource.
         int32_t lenSource = uprv_strlen(source);
-        const UChar *target = NULL, *factor = NULL, *offset = NULL;
-        int32_t lenTarget, lenFactor, lenOffset;
+        const UChar *baseUnit = NULL, *factor = NULL, *offset = NULL;
+        int32_t lenBaseUnit, lenFactor, lenOffset;
         const char *key;
         for (int32_t i = 0; conversionRateTable.getKeyAndValue(i, key, value); ++i) {
             if (uprv_strcmp(key, "target") == 0) {
-                target = value.getString(lenTarget, status);
+                baseUnit = value.getString(lenBaseUnit, status);
             } else if (uprv_strcmp(key, "factor") == 0) {
                 factor = value.getString(lenFactor, status);
             } else if (uprv_strcmp(key, "offset") == 0) {
                 offset = value.getString(lenOffset, status);
             }
         }
-        if (target == NULL || factor == NULL) {
+        if (baseUnit == NULL || factor == NULL) {
             status = U_MISSING_RESOURCE_ERROR;
             return;
         }
@@ -81,19 +81,19 @@ class ConversionRateDataSink : public ResourceSink {
         // Check if we already have the conversion rate in question.
         //
         // TODO(revieW): We could do this skip-check *before* we fetch
-        // target/factor/offset based only on the source unit, but only if we're
-        // certain we'll never get two different targets for a given source.
-        // This should be the case, since convertUnit entries in CLDR's
+        // baseUnit/factor/offset based only on the source unit, but only if
+        // we're certain we'll never get two different baseUnits for a given
+        // source. This should be the case, since convertUnit entries in CLDR's
         // units.xml should all point at a defined base unit for the unit
         // category. I should make this code more efficient after
         // double-checking we're fine with relying on such a detail from the
         // CLDR spec?
-        fLastTarget.clear();
-        fLastTarget.appendInvariantChars(target, lenTarget, status);
+        fLastBaseUnit.clear();
+        fLastBaseUnit.appendInvariantChars(baseUnit, lenBaseUnit, status);
         if (U_FAILURE(status)) return;
         for (int32_t i = 0, len = outVector.length(); i < len; i++) {
             if (strcmp(outVector[i]->source.data(), source) == 0 &&
-                strcmp(outVector[i]->target.data(), fLastTarget.data()) == 0) {
+                strcmp(outVector[i]->baseUnit.data(), fLastBaseUnit.data()) == 0) {
                 return;
             }
         }
@@ -106,30 +106,30 @@ class ConversionRateDataSink : public ResourceSink {
             return;
         } else {
             cr->source.append(source, lenSource, status);
-            cr->target.append(fLastTarget.data(), fLastTarget.length(), status);
+            cr->baseUnit.append(fLastBaseUnit.data(), fLastBaseUnit.length(), status);
             cr->factor.appendInvariantChars(factor, lenFactor, status);
             if (offset != NULL) cr->offset.appendInvariantChars(offset, lenOffset, status);
         }
     }
 
     /**
-     * Returns the MeasureUnit that was the conversion target of the most recent
-     * call to put() - typically meaning the most recent call to
+     * Returns the MeasureUnit that was the conversion base unit of the most
+     * recent call to put() - typically meaning the most recent call to
      * ures_getAllItemsWithFallback().
      */
-    MeasureUnit getLastTarget(UErrorCode &status) {
-        return MeasureUnit::forIdentifier(fLastTarget.data(), status);
+    MeasureUnit getLastBaseUnit(UErrorCode &status) {
+        return MeasureUnit::forIdentifier(fLastBaseUnit.data(), status);
     }
 
   private:
     MaybeStackVector<ConversionRateInfo> &outVector;
 
     // TODO(review): felt like a hack: provides easy access to the most recent
-    // target. This hack is another point making me wonder if doing this
+    // baseUnit. This hack is another point making me wonder if doing this
     // ResourceSink thing is worthwhile. Functional style is not more verbose,
     // and IMHO more readable than this object-based approach where the output
     // seems/feels like a side-effect.
-    CharString fLastTarget;
+    CharString fLastBaseUnit;
 };
 
 // WIP/FIXME: partially due to my skepticism of using the ResourceSink design
@@ -254,7 +254,7 @@ void processSingleUnit(const MeasureUnit &unit, const UResourceBundle *convertUn
     ures_getAllItemsWithFallback(convertUnitsBundle, simple.getIdentifier(), convertSink, status);
 
     if (baseSingleUnit != NULL) {
-        *baseSingleUnit = convertSink.getLastTarget(status).withDimensionality(dimensionality, status);
+        *baseSingleUnit = convertSink.getLastBaseUnit(status).withDimensionality(dimensionality, status);
     }
 }
 
@@ -343,7 +343,7 @@ void getUnitsData(const char *outputRegion, const char *usage, const MeasureUnit
         status = U_MISSING_RESOURCE_ERROR;
         return;
     }
-    const char *baseIdentifier = conversionRates[0]->target.data();
+    const char *baseIdentifier = conversionRates[0]->baseUnit.data();
     baseUnit = MeasureUnit::forIdentifier(baseIdentifier, status);
 
     // category
index 78b985431a7697f60a1103521a3d4ef86df6c342..b1ee1ede6db40a79cb1eecdee31d90e57d9de337 100644 (file)
@@ -21,16 +21,16 @@ U_NAMESPACE_BEGIN
 class U_I18N_API ConversionRateInfo {
   public:
     ConversionRateInfo() {};
-    ConversionRateInfo(StringPiece source, StringPiece target, StringPiece factor, StringPiece offset,
+    ConversionRateInfo(StringPiece source, StringPiece baseUnit, StringPiece factor, StringPiece offset,
                        UErrorCode &status)
-        : source(), target(), factor(), offset() {
+        : source(), baseUnit(), factor(), offset() {
         this->source.append(source, status);
-        this->target.append(target, status);
+        this->baseUnit.append(baseUnit, status);
         this->factor.append(factor, status);
         this->offset.append(offset, status);
     };
     CharString source;
-    CharString target; // FIXME/WIP: baseUnit
+    CharString baseUnit; // FIXME/WIP: baseUnit
     CharString factor;
     CharString offset;
     bool reciprocal = false;
@@ -47,7 +47,7 @@ struct U_I18N_API UnitPreference {
 
 /**
  * Collects and returns ConversionRateInfo needed to convert from source to
- * target.
+ * baseUnit.
  * 
  * @param source The source unit (the unit type converted from).
  * @param target The target unit (the unit type converted to).
index 392e95aa8affd3154fe645a771c08fb7aa8d85b4..7b61eb5a2a88802db87b0228841750ea2039e74f 100644 (file)
@@ -423,7 +423,7 @@ StringPiece getTarget(StringPiece source, const MaybeStackVector<ConversionRateI
                       UErrorCode &status) {
     const auto& convertUnit = extractConversionRateInfo(source, ratesInfo, status);
     if (U_FAILURE(status)) return StringPiece("");
-    return convertUnit.target.toStringPiece();
+    return convertUnit.baseUnit.toStringPiece();
 }
 
 // TODO(ICU-20568): Add more test coverage for this function.
index 2086f458e9faa62586b613f0b1d5211a9987e966..1719147e8ee821ecb1d24e8533c037dd87f8b422 100644 (file)
@@ -610,10 +610,10 @@ void UnitsTest::testGetConversionRateInfo() {
         for (int i = 0; i < conversionInfo.length(); i++) {
             ConversionRateInfo *cri;
             cri = conversionInfo[i];
-            logln("* conversionInfo %d: source=\"%s\", target=\"%s\", factor=\"%s\", offset=\"%s\"", i,
-                  cri->source.data(), cri->target.data(), cri->factor.data(), cri->offset.data());
-            assertTrue("ConversionRateInfo has source, target, and factor",
-                       cri->source.length() > 0 && cri->target.length() > 0 && cri->factor.length() > 0);
+            logln("* conversionInfo %d: source=\"%s\", baseUnit=\"%s\", factor=\"%s\", offset=\"%s\"", i,
+                  cri->source.data(), cri->baseUnit.data(), cri->factor.data(), cri->offset.data());
+            assertTrue("ConversionRateInfo has source, baseUnit, and factor",
+                       cri->source.length() > 0 && cri->baseUnit.length() > 0 && cri->factor.length() > 0);
         }
     }
 }
@@ -661,10 +661,10 @@ void UnitsTest::testGetUnitsData() {
         for (int i = 0; i < conversionInfo.length(); i++) {
             ConversionRateInfo *cri;
             cri = conversionInfo[i];
-            logln("* conversionInfo %d: source=\"%s\", target=\"%s\", factor=\"%s\", offset=\"%s\"", i,
-                  cri->source.data(), cri->target.data(), cri->factor.data(), cri->offset.data());
-            assertTrue("ConversionRateInfo has source, target, and factor",
-                       cri->source.length() > 0 && cri->target.length() > 0 && cri->factor.length() > 0);
+            logln("* conversionInfo %d: source=\"%s\", baseUnit=\"%s\", factor=\"%s\", offset=\"%s\"", i,
+                  cri->source.data(), cri->baseUnit.data(), cri->factor.data(), cri->offset.data());
+            assertTrue("ConversionRateInfo has source, baseUnit, and factor",
+                       cri->source.length() > 0 && cri->baseUnit.length() > 0 && cri->factor.length() > 0);
         }
         assertTrue("at least one unit preference obtained", unitPreferences.length() > 0);
         for (int i = 0; i < unitPreferences.length(); i++) {