From 2138ac8a0ea241b13bf2645ec8b8fa8178e9780c Mon Sep 17 00:00:00 2001 From: Younies Mahmoud Date: Tue, 23 Feb 2021 15:09:01 +0000 Subject: [PATCH] =?UTF-8?q?ICU-21349=20Add=20extra=20UnitsRouter=20constru?= =?UTF-8?q?ctor=20that=20takes=20only=20CLDR=20unit=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See #1579 --- icu4c/source/i18n/units_router.cpp | 17 ++++++++++- icu4c/source/i18n/units_router.h | 6 +++- icu4c/source/test/intltest/units_test.cpp | 29 +++++++++++++++++++ .../com/ibm/icu/impl/units/UnitsRouter.java | 11 ++++--- .../com/ibm/icu/dev/test/impl/UnitsTest.java | 27 +++++++++++++++-- 5 files changed, 81 insertions(+), 9 deletions(-) diff --git a/icu4c/source/i18n/units_router.cpp b/icu4c/source/i18n/units_router.cpp index 9fc389d4395..51f66bfa892 100644 --- a/icu4c/source/i18n/units_router.cpp +++ b/icu4c/source/i18n/units_router.cpp @@ -43,8 +43,23 @@ Precision UnitsRouter::parseSkeletonToPrecision(icu::UnicodeString precisionSkel return result; } -UnitsRouter::UnitsRouter(MeasureUnit inputUnit, StringPiece region, StringPiece usage, +UnitsRouter::UnitsRouter(StringPiece inputUnitIdentifier, StringPiece region, StringPiece usage, UErrorCode &status) { + this->init(MeasureUnit::forIdentifier(inputUnitIdentifier, status), region, usage, status); +} + +UnitsRouter::UnitsRouter(const MeasureUnit &inputUnit, StringPiece region, StringPiece usage, + UErrorCode &status) { + this->init(std::move(inputUnit), region, usage, status); +} + +void UnitsRouter::init(const MeasureUnit &inputUnit, StringPiece region, StringPiece usage, + UErrorCode &status) { + + if (U_FAILURE(status)) { + return; + } + // TODO: do we want to pass in ConversionRates and UnitPreferences instead // of loading in each UnitsRouter instance? (Or make global?) ConversionRates conversionRates(status); diff --git a/icu4c/source/i18n/units_router.h b/icu4c/source/i18n/units_router.h index bd7a93d2d8c..c6e4e4f5288 100644 --- a/icu4c/source/i18n/units_router.h +++ b/icu4c/source/i18n/units_router.h @@ -120,7 +120,9 @@ namespace units { */ class U_I18N_API UnitsRouter { public: - UnitsRouter(MeasureUnit inputUnit, StringPiece locale, StringPiece usage, UErrorCode &status); + UnitsRouter(StringPiece inputUnitIdentifier, StringPiece locale, StringPiece usage, + UErrorCode &status); + UnitsRouter(const MeasureUnit &inputUnit, StringPiece locale, StringPiece usage, UErrorCode &status); /** * Performs locale and usage sensitive unit conversion. @@ -152,6 +154,8 @@ class U_I18N_API UnitsRouter { static number::Precision parseSkeletonToPrecision(icu::UnicodeString precisionSkeleton, UErrorCode &status); + + void init(const MeasureUnit &inputUnit, StringPiece locale, StringPiece usage, UErrorCode &status); }; } // namespace units diff --git a/icu4c/source/test/intltest/units_test.cpp b/icu4c/source/test/intltest/units_test.cpp index 8e9bfbc75ed..87cd16dd60f 100644 --- a/icu4c/source/test/intltest/units_test.cpp +++ b/icu4c/source/test/intltest/units_test.cpp @@ -964,6 +964,35 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie } // TODO: revisit this experimentally chosen precision: checkOutput(unitsTest, msg.data(), expected, routeResult.measures, 0.0000000001); + + // Test UnitsRouter created with CLDR units identifiers. + CharString inputUnitIdentifier(inputUnit, status); + UnitsRouter router2(inputUnitIdentifier.data(), region, usage, status); + if (status.errIfFailureAndReset("UnitsRouter2(<%s>, \"%.*s\", \"%.*s\", status)", + inputUnitIdentifier.data(), region.length(), region.data(), + usage.length(), usage.data())) { + return; + } + + CharString msg2(quantity, status); + msg2.append(" ", status); + msg2.append(usage, status); + msg2.append(" ", status); + msg2.append(region, status); + msg2.append(" ", status); + msg2.append(inputD, status); + msg2.append(" ", status); + msg2.append(inputUnitIdentifier.data(), status); + if (status.errIfFailureAndReset("Failure before router2.route")) { + return; + } + + RouteResult routeResult2 = router2.route(inputAmount, nullptr, status); + if (status.errIfFailureAndReset("router2.route(inputAmount, ...)")) { + return; + } + // TODO: revisit this experimentally chosen precision: + checkOutput(unitsTest, msg2.data(), expected, routeResult.measures, 0.0000000001); } /** diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitsRouter.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitsRouter.java index b7c925c810e..cfb6fa0bca9 100644 --- a/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitsRouter.java +++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/units/UnitsRouter.java @@ -47,13 +47,16 @@ public class UnitsRouter { private ArrayList outputUnits_ = new ArrayList<>(); private ArrayList converterPreferences_ = new ArrayList<>(); - public UnitsRouter(MeasureUnitImpl inputUnitImpl, String region, String usage) { + public UnitsRouter(String inputUnitIdentifier, String region, String usage) { + this(MeasureUnitImpl.forIdentifier(inputUnitIdentifier), region, usage); + } + + public UnitsRouter(MeasureUnitImpl inputUnit, String region, String usage) { // TODO: do we want to pass in ConversionRates and UnitPreferences instead? // of loading in each UnitsRouter instance? (Or make global?) UnitsData data = new UnitsData(); - //MeasureUnitImpl inputUnitImpl = MeasureUnitImpl.forMeasureUnitMaybeCopy(inputUnit); - String category = data.getCategory(inputUnitImpl); + String category = data.getCategory(inputUnit); UnitPreferences.UnitPreference[] unitPreferences = data.getPreferencesFor(category, usage, region); for (int i = 0; i < unitPreferences.length; ++i) { @@ -74,7 +77,7 @@ public class UnitsRouter { } outputUnits_.add(complexTargetUnitImpl.build()); - converterPreferences_.add(new ConverterPreference(inputUnitImpl, complexTargetUnitImpl, + converterPreferences_.add(new ConverterPreference(inputUnit, complexTargetUnitImpl, preference.getGeq(), precision, data.getConversionRates())); } diff --git a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/impl/UnitsTest.java b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/impl/UnitsTest.java index 107503a247e..927ff58cadf 100644 --- a/icu4j/main/tests/core/src/com/ibm/icu/dev/test/impl/UnitsTest.java +++ b/icu4j/main/tests/core/src/com/ibm/icu/dev/test/impl/UnitsTest.java @@ -646,8 +646,7 @@ public class UnitsTest { } } - for (TestCase testCase : - tests) { + for (TestCase testCase : tests) { UnitsRouter router = new UnitsRouter(testCase.inputUnit.second, testCase.region, testCase.usage); List measures = router.route(testCase.input, null).complexConverterResult.measures; @@ -661,10 +660,32 @@ public class UnitsTest { if (!UnitsTest .compareTwoBigDecimal(testCase.expectedInOrder.get(i), BigDecimal.valueOf(measures.get(i).getNumber().doubleValue()), - BigDecimal.valueOf(0.00001))) { + BigDecimal.valueOf(0.0000000001))) { fail("Test failed: " + testCase + "; Got unexpected result: " + measures); } } } + + // Test UnitsRouter created with CLDR units identifiers. + for (TestCase testCase : tests) { + UnitsRouter router = new UnitsRouter(testCase.inputUnit.first, testCase.region, testCase.usage); + List measures = router.route(testCase.input, null).complexConverterResult.measures; + + assertEquals("Measures size must be the same as expected units", + measures.size(), testCase.expectedInOrder.size()); + assertEquals("Measures size must be the same as output units", + measures.size(), testCase.outputUnitInOrder.size()); + + + for (int i = 0; i < measures.size(); i++) { + if (!UnitsTest + .compareTwoBigDecimal(testCase.expectedInOrder.get(i), + BigDecimal.valueOf(measures.get(i).getNumber().doubleValue()), + BigDecimal.valueOf(0.0000000001))) { + fail("Test failed: " + testCase + "; Got unexpected result: " + measures); + } + } + } + } }