From bf71184800ad9aaaff49489f3d2380d62d8b0d10 Mon Sep 17 00:00:00 2001 From: Younies Mahmoud Date: Mon, 8 Jun 2020 19:52:07 +0200 Subject: [PATCH] fix units router and allow the test cases to compare double numbers with precision --- icu4c/source/i18n/complexunitsconverter.cpp | 17 +++++++++-------- icu4c/source/i18n/unitsrouter.cpp | 8 ++++++-- icu4c/source/test/intltest/intltest.cpp | 6 +++++- icu4c/source/test/intltest/unitstest.cpp | 19 +++++++++++++------ 4 files changed, 33 insertions(+), 17 deletions(-) diff --git a/icu4c/source/i18n/complexunitsconverter.cpp b/icu4c/source/i18n/complexunitsconverter.cpp index b2324104101..5739f86048d 100644 --- a/icu4c/source/i18n/complexunitsconverter.cpp +++ b/icu4c/source/i18n/complexunitsconverter.cpp @@ -24,7 +24,8 @@ ComplexUnitsConverter::ComplexUnitsConverter(const MeasureUnit inputUnit, const singleUnitsInOrder.emplaceBack(singleUnits[i]); } - ComplexUnitsConverter(inputUnit, std::move(singleUnitsInOrder), ratesInfo, status); + *this = ComplexUnitsConverter(inputUnit, std::move(singleUnitsInOrder), + ratesInfo, status); // TODO(younies): question from Hugo: is this check appropriate? The // U_ASSERT in greaterThanOrEqual suggests this should be an invariant for @@ -42,21 +43,21 @@ ComplexUnitsConverter::ComplexUnitsConverter(const MeasureUnit inputUnit, return; } - MaybeStackVector converters; for (int i = 0, n = outputUnits.length(); i < n; i++) { - if (i == 0) { // first element - converters.emplaceBack(inputUnit, *outputUnits[i], ratesInfo, status); + if (i == 0) { // first element + unitConverters_.emplaceBack(inputUnit, *outputUnits[i], ratesInfo, + status); - } else { - converters.emplaceBack(*outputUnits[i - 1], *outputUnits[i], ratesInfo, status); - } + } else { + unitConverters_.emplaceBack(*outputUnits[i - 1], *outputUnits[i], + ratesInfo, status); + } if (U_FAILURE(status)) break; } if (U_FAILURE(status)) return; - unitConverters_.appendAll(converters, status); units_.appendAll(outputUnits, status); } diff --git a/icu4c/source/i18n/unitsrouter.cpp b/icu4c/source/i18n/unitsrouter.cpp index a180220023b..dfab7929a67 100644 --- a/icu4c/source/i18n/unitsrouter.cpp +++ b/icu4c/source/i18n/unitsrouter.cpp @@ -38,8 +38,12 @@ UnitsRouter::UnitsRouter(MeasureUnit inputUnit, StringPiece region, StringPiece const auto &preference = *unitPreferences[i]; MeasureUnit complexTargetUnit = MeasureUnit::forIdentifier(preference.unit.data(), status); - if (U_FAILURE(status)) { return; } - converterPreferences_.emplaceBack(inputUnit, complexTargetUnit, preference.geq, conversionRates, + if (U_FAILURE(status)) { + return; + } + + converterPreferences_.emplaceBack(inputUnit, complexTargetUnit, + preference.geq, conversionRates, status); if (U_FAILURE(status)) { fprintf( diff --git a/icu4c/source/test/intltest/intltest.cpp b/icu4c/source/test/intltest/intltest.cpp index 7c6d7a6cd89..993222b2448 100644 --- a/icu4c/source/test/intltest/intltest.cpp +++ b/icu4c/source/test/intltest/intltest.cpp @@ -2177,7 +2177,11 @@ UBool IntlTest::assertNotEquals(const char* message, // http://junit.sourceforge.net/javadoc/org/junit/Assert.html#assertEquals(java.lang.String,%20double,%20double,%20double) UBool IntlTest::assertEqualsNear(const char *message, double expected, double actual, double precision) { double diff = std::abs(expected - actual); - double diffPercent = expected != 0? diff / expected : diff; // If the expected is equals zero, we + double diffPercent = + expected != 0 ? diff / expected + : diff; // If the expected is equals zero, we assume that + // the `diffPercent` is equal to the difference + // between the actual and the expected if (diffPercent > precision) { errln((UnicodeString) "FAIL: " + message + "; got " + actual + "; expected " + expected); diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp index ca8e3bd1f8c..bed4501ec3d 100644 --- a/icu4c/source/test/intltest/unitstest.cpp +++ b/icu4c/source/test/intltest/unitstest.cpp @@ -6,6 +6,7 @@ #if !UCONFIG_NO_FORMATTING #include +#include #include "charstr.h" #include "cmemory.h" @@ -502,7 +503,7 @@ class ExpectedOutput { _skippedFields++; if (_skippedFields < 2) { // We are happy skipping one field per output unit: we want to skip - // rational fraction fiels like "11 / 10". + // rational fraction files like "11 / 10". errorCode = U_ZERO_ERROR; return; } else { @@ -527,7 +528,7 @@ class ExpectedOutput { }; void checkOutput(UnitsTest *unitsTest, const char *msg, ExpectedOutput expected, - const MaybeStackVector &actual) { + const MaybeStackVector &actual, double precision) { IcuTestErrorCode status(*unitsTest, "checkOutput"); bool success = true; if (expected._compoundCount != actual.length()) { @@ -537,10 +538,16 @@ void checkOutput(UnitsTest *unitsTest, const char *msg, ExpectedOutput expected, if (i >= expected._compoundCount) { break; } - if (expected._amounts[i] != actual[i]->getNumber().getDouble(status)) { - success = false; - break; + + double diff = std::abs(expected._amounts[i] - + actual[i]->getNumber().getDouble(status)); + double diffPercent = + expected._amounts[i] != 0 ? diff / expected._amounts[i] : diff; + if (diffPercent > precision) { + success = false; + break; } + if (expected._measureUnits[i] != actual[i]->getUnit()) { success = false; break; @@ -633,7 +640,7 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie if (status.errIfFailureAndReset("Failure before router.route")) { return; } MaybeStackVector result = router.route(inputAmount, status); if (status.errIfFailureAndReset("router.route(inputAmount, ...)")) { return; } - checkOutput(unitsTest, msg.data(), expected, result); + checkOutput(unitsTest, msg.data(), expected, result, 0.0001); } /** -- 2.40.0