From 24b50c73a969ab20034c5d773abd9071a6ff1058 Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Mon, 11 May 2020 22:17:50 +0200 Subject: [PATCH] Use UnitPreferences, drop getUnitsData and related. --- icu4c/source/i18n/unitsdata.cpp | 123 ----------------------- icu4c/source/i18n/unitsrouter.cpp | 13 ++- icu4c/source/test/intltest/unitstest.cpp | 65 ------------ 3 files changed, 6 insertions(+), 195 deletions(-) diff --git a/icu4c/source/i18n/unitsdata.cpp b/icu4c/source/i18n/unitsdata.cpp index 47b9b811ef7..9c7b18c0a7d 100644 --- a/icu4c/source/i18n/unitsdata.cpp +++ b/icu4c/source/i18n/unitsdata.cpp @@ -108,67 +108,6 @@ class ConversionRateDataSink : public ResourceSink { MaybeStackVector *outVector; }; -/** - * Collects unit preference information from a set of preferences. - * @param usageData This should be a resource bundle containing a vector of - * preferences - i.e. the unitPreferenceData tree resources already narrowed - * down to a particular usage and region (example: - * "unitPreferenceData/length/road/GB"). - */ -void collectUnitPrefs(UResourceBundle *usageData, MaybeStackVector &outVector, - UErrorCode &status) { - if (U_FAILURE(status)) return; - StackUResourceBundle prefBundle; - - int32_t numPrefs = ures_getSize(usageData); - for (int32_t i = 0; i < numPrefs; i++) { - ures_getByIndex(usageData, i, prefBundle.getAlias(), &status); - - // Add and populate a new UnitPreference - int32_t strLen; - - // unit - const UChar *unitIdent = ures_getStringByKey(prefBundle.getAlias(), "unit", &strLen, &status); - if (U_FAILURE(status)) return; - UnitPreference *up = outVector.emplaceBack(); - if (!up) { - status = U_MEMORY_ALLOCATION_ERROR; - return; - } - up->unit.appendInvariantChars(unitIdent, strLen, status); - - // geq - const UChar *geq = ures_getStringByKey(prefBundle.getAlias(), "geq", &strLen, &status); - if (U_SUCCESS(status)) { - // If we don't mind up->geq having a bad value when - // U_FAILURE(status), we could extract a function and do a one-liner: - // up->geq = UCharsToDouble(geq, status); - CharString cGeq; - cGeq.appendInvariantChars(geq, strLen, status); - DecimalQuantity dq; - dq.setToDecNumber(StringPiece(cGeq.data()), status); - if (U_FAILURE(status)) return; - up->geq = dq.toDouble(); - } else if (status == U_MISSING_RESOURCE_ERROR) { - // We don't mind if geq is missing - status = U_ZERO_ERROR; - } else { - return; - } - - // skeleton - const UChar *skel = ures_getStringByKey(prefBundle.getAlias(), "skeleton", &strLen, &status); - if (U_SUCCESS(status)) { - up->skeleton.appendInvariantChars(skel, strLen, status); - } else if (status == U_MISSING_RESOURCE_ERROR) { - // We don't mind if geq is missing - status = U_ZERO_ERROR; - } else { - return; - } - } -} - UnitPreferenceMetadata::UnitPreferenceMetadata(const char *category, const char *usage, const char *region, int32_t prefsOffset, int32_t prefsCount, UErrorCode &status) { @@ -436,68 +375,6 @@ const ConversionRateInfo *ConversionRates::extractConversionInfo(StringPiece sou return nullptr; } -/** - * Fetches the units data that would be needed for the given usage. - * - * @param inputUnit the unit for which input is expected. (NOTE/WIP: If this is - * known to be a base unit already, we could strip some logic here.) - */ -void getUnitsData(const char *outputRegion, const char *usage, const MeasureUnit &inputUnit, - CharString &category, MeasureUnit &baseUnit, - MaybeStackVector &conversionRates, - MaybeStackVector &unitPreferences, UErrorCode &status) { - // This first fetches all conversion info. Next it fetches the category and - // unit preferences for the given usage and region. - - // In this function we use LocalUResourceBundlePointers for resource bundles - // that don't change, and StackUResourceBundles for structures we use as - // fillin. - - getAllConversionRates(conversionRates, status); - if (U_FAILURE(status)) return; - if (conversionRates.length() < 1) { - // This is defensive programming, because this shouldn't happen: if - // convertSink succeeds, there should be at least one item in - // conversionRates. - status = U_MISSING_RESOURCE_ERROR; - return; - } - // TODO(hugovdm): this is broken. We fetch all conversion rates now, the - // first is nothing special. - const char *baseIdentifier = conversionRates[0]->baseUnit.data(); - baseUnit = MeasureUnit::forIdentifier(baseIdentifier, status); - - // find category - LocalUResourceBundlePointer unitsBundle(ures_openDirect(NULL, "units", &status)); - LocalUResourceBundlePointer unitQuantities( - ures_getByKey(unitsBundle.getAlias(), "unitQuantities", NULL, &status)); - int32_t categoryLength; - const UChar *uCategory = - ures_getStringByKey(unitQuantities.getAlias(), baseIdentifier, &categoryLength, &status); - category.appendInvariantChars(uCategory, categoryLength, status); - - // Find the right unit preference bundle - StackUResourceBundle stackBundle; // Reused as we climb the tree - ures_getByKey(unitsBundle.getAlias(), "unitPreferenceData", stackBundle.getAlias(), &status); - ures_getByKey(stackBundle.getAlias(), category.data(), stackBundle.getAlias(), &status); - if (U_FAILURE(status)) { return; } - ures_getByKey(stackBundle.getAlias(), usage, stackBundle.getAlias(), &status); - if (status == U_MISSING_RESOURCE_ERROR) { - // Requested usage does not exist, so we use "default". - status = U_ZERO_ERROR; - ures_getByKey(stackBundle.getAlias(), "default", stackBundle.getAlias(), &status); - } - ures_getByKey(stackBundle.getAlias(), outputRegion, stackBundle.getAlias(), &status); - if (status == U_MISSING_RESOURCE_ERROR) { - // Requested region does not exist, so we use "001". - status = U_ZERO_ERROR; - ures_getByKey(stackBundle.getAlias(), "001", stackBundle.getAlias(), &status); - } - - // Collect all the preferences into unitPreferences - collectUnitPrefs(stackBundle.getAlias(), unitPreferences, status); -} - U_I18N_API UnitPreferences::UnitPreferences(UErrorCode &status) { LocalUResourceBundlePointer unitsBundle(ures_openDirect(NULL, "units", &status)); UnitPreferencesSink sink(&unitPrefs_, &metadata_); diff --git a/icu4c/source/i18n/unitsrouter.cpp b/icu4c/source/i18n/unitsrouter.cpp index 5ca1347aeda..381bfdbd3bb 100644 --- a/icu4c/source/i18n/unitsrouter.cpp +++ b/icu4c/source/i18n/unitsrouter.cpp @@ -23,17 +23,16 @@ UnitsRouter::UnitsRouter(MeasureUnit inputUnit, StringPiece locale, StringPiece // MaybeStackVector preferences = extractUnitPreferences(locale, usage, // unitCategory); const char *region = "001"; // FIXME extract from locale. - CharString category; + const char *category = "length"; // FIXME(hugovdm) extract from inputUnit. MeasureUnit baseUnit; ConversionRates conversionRates(status); + UnitPreferences prefs(status); - // WIP/TODO(hugovdm): drop tmpConversionRates, redo getUnitsData. - MaybeStackVector tmpConversionRates; - MaybeStackVector unitPreferences; - getUnitsData(region, usage.data(), inputUnit, category, baseUnit, tmpConversionRates, unitPreferences, - status); + const UnitPreference *const *unitPreferences; + int32_t preferencesCount; + prefs.getPreferencesFor(category, usage.data(), region, unitPreferences, preferencesCount, status); - for (int i = 0, n = unitPreferences.length(); i < n; ++i) { + for (int i = 0; i < preferencesCount; ++i) { const auto &preference = *unitPreferences[i]; MeasureUnit complexTargetUnit = MeasureUnit::forIdentifier(preference.unit.data(), status); diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp index 0c38a1d59d6..46abece8da4 100644 --- a/icu4c/source/test/intltest/unitstest.cpp +++ b/icu4c/source/test/intltest/unitstest.cpp @@ -39,7 +39,6 @@ class UnitsTest : public IntlTest { void testConversionCapability(); void testConversions(); void testPreferences(); - void testGetUnitsData(); void testBasic(); void testSiPrefixes(); @@ -61,7 +60,6 @@ void UnitsTest::runIndexedTest(int32_t index, UBool exec, const char *&name, cha TESTCASE_AUTO(testConversionCapability); TESTCASE_AUTO(testConversions); TESTCASE_AUTO(testPreferences); - TESTCASE_AUTO(testGetUnitsData); TESTCASE_AUTO(testBasic); TESTCASE_AUTO(testSiPrefixes); @@ -702,69 +700,6 @@ void UnitsTest::testPreferences() { } } -// We test "successfully loading some data", not specific output values, since -// this would duplicate some of the input data. We leave end-to-end testing to -// take care of that. Running `intltest` with `-v` will print out the loaded -// output for easy visual inspection. -// -// WIP/TODO(hugovdm): getUnitsData is currently broken. Show it in the test an -// fix it! -void UnitsTest::testGetUnitsData() { - struct { - // Input parameters - const char *outputRegion; - const char *usage; - const char *inputUnit; - } testCases[]{ - {"US", "fluid", "centiliter"}, - {"BZ", "weather", "celsius"}, - {"ZA", "road", "yard"}, - {"XZ", "zz_nonexistant", "dekagram"}, - }; - for (const auto &t : testCases) { - logln("---testing: region=\"%s\", usage=\"%s\", inputUnit=\"%s\"", t.outputRegion, t.usage, - t.inputUnit); - IcuTestErrorCode status(*this, "testGetUnitsData"); - MeasureUnit inputUnit = MeasureUnit::forIdentifier(t.inputUnit, status); - if (status.errIfFailureAndReset("MeasureUnit::forIdentifier(\"%s\", ...)", t.inputUnit)) { - continue; - } - - CharString category; - MeasureUnit baseUnit; - MaybeStackVector conversionInfo; - MaybeStackVector unitPreferences; - getUnitsData(t.outputRegion, t.usage, inputUnit, category, baseUnit, conversionInfo, - unitPreferences, status); - if (status.errIfFailureAndReset("getUnitsData(\"%s\", \"%s\", \"%s\", ...)", t.outputRegion, - t.usage, t.inputUnit)) { - continue; - } - - logln("* category: \"%s\", baseUnit: \"%s\"", category.data(), baseUnit.getIdentifier()); - assertTrue("category filled in", category.length() > 0); - assertTrue("baseUnit filled in", uprv_strlen(baseUnit.getIdentifier()) > 0); - assertTrue("at least one conversion rate obtained", conversionInfo.length() > 0); - for (int i = 0; i < conversionInfo.length(); i++) { - ConversionRateInfo *cri; - cri = conversionInfo[i]; - logln("* conversionInfo %d: source=\"%s\", baseUnit=\"%s\", factor=\"%s\", offset=\"%s\"", i, - cri->sourceUnit.data(), cri->baseUnit.data(), cri->factor.data(), cri->offset.data()); - assertTrue("ConversionRateInfo has source, baseUnit, and factor", - cri->sourceUnit.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++) { - UnitPreference *up; - up = unitPreferences[i]; - logln("* unitPreference %d: \"%s\", geq=%f, skeleton=\"%s\"", i, up->unit.data(), up->geq, - up->skeleton.data()); - assertTrue("unitPreference has unit", up->unit.length() > 0); - } - } -} - /** * Tests different return statuses depending on the input. */ -- 2.40.0