From c49cb7350913ae335c9192a12f584a6b01303f7e Mon Sep 17 00:00:00 2001 From: younies Date: Mon, 30 Mar 2020 14:43:50 +0200 Subject: [PATCH] ICU-20568 Check convertible units PR: https://github.com/sffc/icu/pull/32 Commit: 3bf35258f42f8aca893f6ebfb9a019c2e1ee5563 --- icu4c/source/i18n/unitconverter.cpp | 61 ++++++++++- icu4c/source/i18n/unitconverter.h | 17 +++- icu4c/source/i18n/unitsdata.cpp | 15 +-- icu4c/source/i18n/unitsdata.h | 30 ++++-- icu4c/source/test/depstest/dependencies.txt | 6 +- icu4c/source/test/intltest/unitstest.cpp | 107 +++++++++++++++++--- 6 files changed, 198 insertions(+), 38 deletions(-) diff --git a/icu4c/source/i18n/unitconverter.cpp b/icu4c/source/i18n/unitconverter.cpp index 0e281344bf0..cacf2dba813 100644 --- a/icu4c/source/i18n/unitconverter.cpp +++ b/icu4c/source/i18n/unitconverter.cpp @@ -5,15 +5,74 @@ #if !UCONFIG_NO_FORMATTING +#include "charstr.h" +#include "measunit_impl.h" +#include "unicode/errorcode.h" +#include "unicode/measunit.h" +#include "unicode/stringpiece.h" +#include "unitconverter.h" + U_NAMESPACE_BEGIN +namespace { +/** + * Extracts the compound base unit of a compound unit (`source`). For example, if the source unit is + * `square-mile-per-hour`, the compound base unit will be `square-meter-per-second` + */ +MeasureUnit extractCompoundBaseUnit(const MeasureUnit &source, const ConversionRates &conversionRates, + UErrorCode &status) { + MeasureUnit result; + int32_t count; + const auto singleUnits = source.splitToSingleUnits(count, status); + if (U_FAILURE(status)) return result; + for (int i = 0; i < count; ++i) { + const auto &singleUnit = singleUnits[i]; + // Extract `ConversionRateInfo` using the absolute unit. For example: in case of `square-meter`, + // we will use `meter` + const auto singleUnitImpl = SingleUnitImpl::forMeasureUnit(singleUnit, status); + const auto rateInfo = conversionRates.extractConversionInfo(singleUnitImpl.getSimpleUnitID(), status); + if (U_FAILURE(status)) return result; + if (rateInfo == nullptr) { + status = U_INTERNAL_PROGRAM_ERROR; + return result; + } + // Multiply the power of the singleUnit by the power of the baseUnit. For example, square-hectare + // must be p4-meter. (NOTE: hectare --> square-meter) + auto compoundBaseUnit = MeasureUnit::forIdentifier(rateInfo->baseUnit.toStringPiece(), status); + int32_t baseUnitsCount; + auto baseUnits = compoundBaseUnit.splitToSingleUnits(baseUnitsCount, status); + for (int j = 0; j < baseUnitsCount; j++) { + int8_t newDimensionality = + baseUnits[j].getDimensionality(status) * singleUnit.getDimensionality(status); + result = result.product(baseUnits[j].withDimensionality(newDimensionality, status), status); + if (U_FAILURE(status)) { return result; } + } + } + return result; +} +} // namespace + +UnitsConvertibilityState U_I18N_API checkConvertibility(const MeasureUnit &source, + const MeasureUnit &target, + const ConversionRates &conversionRates, + UErrorCode &status) { + auto sourceBaseUnit = extractCompoundBaseUnit(source, conversionRates, status); + auto targetBaseUnit = extractCompoundBaseUnit(target, conversionRates, status); + + if (U_FAILURE(status)) return UNCONVERTIBLE; + + if (sourceBaseUnit == targetBaseUnit) return CONVERTIBLE; + if (sourceBaseUnit == targetBaseUnit.reciprocal(status)) return RECIPROCAL; + + return UNCONVERTIBLE; +} U_NAMESPACE_END -#endif /* #if !UCONFIG_NO_FORMATTING */ \ No newline at end of file +#endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/i18n/unitconverter.h b/icu4c/source/i18n/unitconverter.h index 9a112f2f0a5..a7c70c43e54 100644 --- a/icu4c/source/i18n/unitconverter.h +++ b/icu4c/source/i18n/unitconverter.h @@ -7,16 +7,27 @@ #ifndef __UNITCONVERTER_H__ #define __UNITCONVERTER_H__ - -#include +#include "cmemory.h" +#include "unicode/errorcode.h" +#include "unicode/measunit.h" +#include "unitconverter.h" +#include "unitsdata.h" U_NAMESPACE_BEGIN +enum U_I18N_API UnitsConvertibilityState { + RECIPROCAL, + CONVERTIBLE, + UNCONVERTIBLE, +}; +UnitsConvertibilityState U_I18N_API checkConvertibility(const MeasureUnit &source, + const MeasureUnit &target, + const ConversionRates &conversionRates, + UErrorCode &status); U_NAMESPACE_END - #endif //__UNITCONVERTER_H__ #endif /* #if !UCONFIG_NO_FORMATTING */ diff --git a/icu4c/source/i18n/unitsdata.cpp b/icu4c/source/i18n/unitsdata.cpp index 5ad29217e3c..c92e151604a 100644 --- a/icu4c/source/i18n/unitsdata.cpp +++ b/icu4c/source/i18n/unitsdata.cpp @@ -102,13 +102,14 @@ void U_I18N_API getAllConversionRates(MaybeStackVector &resu ures_getAllItemsWithFallback(unitsBundle.getAlias(), "convertUnits", sink, status); } -// TODO(hugovdm): ensure this gets removed. Currently -// https://github.com/sffc/icu/pull/32 is making use of it. -MaybeStackVector U_I18N_API -getConversionRatesInfo(const MaybeStackVector &, UErrorCode &status) { - MaybeStackVector result; - getAllConversionRates(result, status); - return result; +const ConversionRateInfo *ConversionRates::extractConversionInfo(StringPiece source, + UErrorCode &status) const { + for (size_t i = 0, n = conversionInfo_.length(); i < n; ++i) { + if (conversionInfo_[i]->sourceUnit.toStringPiece() == source) return conversionInfo_[i]; + } + + status = U_INTERNAL_PROGRAM_ERROR; + return nullptr; } U_NAMESPACE_END diff --git a/icu4c/source/i18n/unitsdata.h b/icu4c/source/i18n/unitsdata.h index 5f4306dc012..99383574c7b 100644 --- a/icu4c/source/i18n/unitsdata.h +++ b/icu4c/source/i18n/unitsdata.h @@ -49,16 +49,28 @@ class U_I18N_API ConversionRateInfo : public UMemory { void U_I18N_API getAllConversionRates(MaybeStackVector &result, UErrorCode &status); /** - * Temporary backward-compatibility function. - * - * TODO(hugovdm): ensure this gets removed. Currently - * https://github.com/sffc/icu/pull/32 is making use of it. - * - * @param units Ignored. - * @return the result of getAllConversionRates. + * Contains all the supported conversion rates. */ -MaybeStackVector - U_I18N_API getConversionRatesInfo(const MaybeStackVector &units, UErrorCode &status); +class U_I18N_API ConversionRates { + public: + /** + * Constructor + * + * @param status Receives status. + */ + ConversionRates(UErrorCode &status) { getAllConversionRates(conversionInfo_, status); } + + /** + * Returns a pointer to the conversion rate info that match the `source`. + * + * @param source Contains the source. + * @param status Receives status. + */ + const ConversionRateInfo *extractConversionInfo(StringPiece source, UErrorCode &status) const; + + private: + MaybeStackVector conversionInfo_; +}; U_NAMESPACE_END diff --git a/icu4c/source/test/depstest/dependencies.txt b/icu4c/source/test/depstest/dependencies.txt index 8ee0acf1288..04e6485bf77 100644 --- a/icu4c/source/test/depstest/dependencies.txt +++ b/icu4c/source/test/depstest/dependencies.txt @@ -1064,7 +1064,7 @@ group: sharedbreakiterator breakiterator group: units_extra - measunit_extra.o unitconverter.o + measunit_extra.o deps units bytestriebuilder bytestrie resourcebundle uclean_i18n @@ -1074,9 +1074,9 @@ group: units stringenumeration errorcode group: unitsformatter - unitsdata.o + unitsdata.o unitconverter.o deps - resourcebundle + resourcebundle units_extra group: decnumber decContext.o decNumber.o diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp index 2bca6d64beb..1f85e88f925 100644 --- a/icu4c/source/test/intltest/unitstest.cpp +++ b/icu4c/source/test/intltest/unitstest.cpp @@ -6,6 +6,7 @@ #if !UCONFIG_NO_FORMATTING #include "charstr.h" +#include "cmemory.h" #include "filestrm.h" #include "intltest.h" #include "number_decimalquantity.h" @@ -13,6 +14,8 @@ #include "unicode/measunit.h" #include "unicode/unistr.h" #include "unicode/unum.h" +#include "unitconverter.h" +#include "unitsdata.h" #include "uparse.h" using icu::number::impl::DecimalQuantity; @@ -23,6 +26,7 @@ class UnitsTest : public IntlTest { void runIndexedTest(int32_t index, UBool exec, const char *&name, char *par = NULL); + void testConversionCapability(); void testConversions(); void testPreferences(); // void testBasic(); @@ -35,10 +39,9 @@ class UnitsTest : public IntlTest { extern IntlTest *createUnitsTest() { return new UnitsTest(); } void UnitsTest::runIndexedTest(int32_t index, UBool exec, const char *&name, char * /*par*/) { - if (exec) { - logln("TestSuite UnitsTest: "); - } + if (exec) { logln("TestSuite UnitsTest: "); } TESTCASE_AUTO_BEGIN; + TESTCASE_AUTO(testConversionCapability); TESTCASE_AUTO(testConversions); TESTCASE_AUTO(testPreferences); // TESTCASE_AUTO(testBasic); @@ -51,15 +54,83 @@ void UnitsTest::runIndexedTest(int32_t index, UBool exec, const char *&name, cha // Just for testing quick conversion ability. double testConvert(UnicodeString source, UnicodeString target, double input) { - if (source == u"meter" && target == u"foot" && input == 1.0) - return 3.28084; + if (source == u"meter" && target == u"foot" && input == 1.0) return 3.28084; - if ( source == u"kilometer" && target == u"foot" && input == 1.0) - return 328.084; + if (source == u"kilometer" && target == u"foot" && input == 1.0) return 328.084; return -1; } +void UnitsTest::testConversionCapability() { + struct TestCase { + const StringPiece source; + const StringPiece target; + const UnitsConvertibilityState expectedState; + } testCases[]{ + {"meter", "foot", CONVERTIBLE}, // + {"kilometer", "foot", CONVERTIBLE}, // + {"hectare", "square-foot", CONVERTIBLE}, // + {"kilometer-per-second", "second-per-meter", RECIPROCAL}, // + {"square-meter", "square-foot", CONVERTIBLE}, // + {"kilometer-per-second", "foot-per-second", CONVERTIBLE}, // + {"square-hectare", "p4-foot", CONVERTIBLE}, // + {"square-kilometer-per-second", "second-per-square-meter", RECIPROCAL}, // + // TODO: Remove the following test cases after hocking up unitsTest.txt. + {"g-force", "meter-per-square-second", CONVERTIBLE}, // + {"ohm", "kilogram-square-meter-per-cubic-second-square-ampere", CONVERTIBLE}, // + {"electronvolt", "kilogram-square-meter-per-square-second", CONVERTIBLE}, // + {"dalton", "kilogram-square-meter-per-square-second", CONVERTIBLE}, // + {"joule", "kilogram-square-meter-per-square-second", CONVERTIBLE}, // + {"meter-newton", "kilogram-square-meter-per-square-second", CONVERTIBLE}, // + {"foot-pound-force", "kilogram-square-meter-per-square-second", CONVERTIBLE}, // + {"calorie", "kilogram-square-meter-per-square-second", CONVERTIBLE}, // + {"kilojoule", "kilogram-square-meter-per-square-second", CONVERTIBLE}, // + {"british-thermal-unit", "kilogram-square-meter-per-square-second", CONVERTIBLE}, // + {"foodcalorie", "kilogram-square-meter-per-square-second", CONVERTIBLE}, // + {"kilocalorie", "kilogram-square-meter-per-square-second", CONVERTIBLE}, // + {"hour-kilowatt", "kilogram-square-meter-second-per-cubic-second", CONVERTIBLE}, // + {"therm-us", "kilogram-square-meter-per-square-second", CONVERTIBLE}, // + {"newton", "kilogram-meter-per-square-second", CONVERTIBLE}, // + {"pound-force", "kilogram-meter-per-square-second", CONVERTIBLE}, // + {"hertz", "revolution-per-second", CONVERTIBLE}, // + {"kilohertz", "revolution-per-second", CONVERTIBLE}, // + {"megahertz", "revolution-per-second", CONVERTIBLE}, // + {"gigahertz", "revolution-per-second", CONVERTIBLE}, // + {"lux", "candela-square-meter-per-square-meter", CONVERTIBLE}, // + {"milliwatt", "kilogram-square-meter-per-cubic-second", CONVERTIBLE}, // + {"watt", "kilogram-square-meter-per-cubic-second", CONVERTIBLE}, // + {"horsepower", "kilogram-square-meter-per-cubic-second", CONVERTIBLE}, // + {"kilowatt", "kilogram-square-meter-per-cubic-second", CONVERTIBLE}, // + {"megawatt", "kilogram-square-meter-per-cubic-second", CONVERTIBLE}, // + {"gigawatt", "kilogram-square-meter-per-cubic-second", CONVERTIBLE}, // + {"solar-luminosity", "kilogram-square-meter-per-cubic-second", CONVERTIBLE}, // + {"pascal", "kilogram-per-meter-square-second", CONVERTIBLE}, // + {"hectopascal", "kilogram-per-meter-square-second", CONVERTIBLE}, // + {"millibar", "kilogram-per-meter-square-second", CONVERTIBLE}, // + {"millimeter-ofhg", "kilogram-meter-per-square-meter-square-second", CONVERTIBLE}, // + {"kilopascal", "kilogram-per-meter-square-second", CONVERTIBLE}, // + {"inch-ofhg", "kilogram-meter-per-square-meter-square-second", CONVERTIBLE}, // + {"bar", "kilogram-per-meter-square-second", CONVERTIBLE}, // + {"atmosphere", "kilogram-per-meter-square-second", CONVERTIBLE}, // + {"megapascal", "kilogram-per-meter-square-second", CONVERTIBLE}, // + {"ofhg", "kilogram-per-square-meter-square-second", CONVERTIBLE}, // + {"knot", "meter-per-second", CONVERTIBLE}, // + {"volt", "kilogram-square-meter-per-cubic-second-ampere", CONVERTIBLE}, // + }; + + for (const auto &testCase : testCases) { + UErrorCode status = U_ZERO_ERROR; + + MeasureUnit source = MeasureUnit::forIdentifier(testCase.source, status); + MeasureUnit target = MeasureUnit::forIdentifier(testCase.target, status); + + ConversionRates conversionRates(status); + auto convertibility = icu::checkConvertibility(source, target, conversionRates, status); + + assertEquals("Conversion Capability", testCase.expectedState, convertibility); + } +} + // void UnitsTest::testBasic() { // IcuTestErrorCode status(*this, "Units testBasic"); @@ -72,7 +143,8 @@ double testConvert(UnicodeString source, UnicodeString target, double input) { // } testCases[]{{u"meter", u"foot", 1.0, 3.28084}, {u"kilometer", u"foot", 1.0, 328.084}}; // for (const auto &testCase : testCases) { -// assertEquals("test convert", testConvert(testCase.source, testCase.target, testCase.inputValue), +// assertEquals("test convert", testConvert(testCase.source, testCase.target, +// testCase.inputValue), // testCase.expectedValue); // } // } @@ -95,7 +167,8 @@ double testConvert(UnicodeString source, UnicodeString target, double input) { // }; // for (const auto &testCase : testCases) { -// assertEquals("test convert", testConvert(testCase.source, testCase.target, testCase.inputValue), +// assertEquals("test convert", testConvert(testCase.source, testCase.target, +// testCase.inputValue), // testCase.expectedValue); // } // } @@ -121,7 +194,8 @@ double testConvert(UnicodeString source, UnicodeString target, double input) { // }; // for (const auto &testCase : testCases) { -// assertEquals("test convert", testConvert(testCase.source, testCase.target, testCase.inputValue), +// assertEquals("test convert", testConvert(testCase.source, testCase.target, +// testCase.inputValue), // testCase.expectedValue); // } // } @@ -146,7 +220,8 @@ double testConvert(UnicodeString source, UnicodeString target, double input) { // }; // for (const auto &testCase : testCases) { -// assertEquals("test convert", testConvert(testCase.source, testCase.target, testCase.inputValue), +// assertEquals("test convert", testConvert(testCase.source, testCase.target, +// testCase.inputValue), // testCase.expectedValue); // } // } @@ -167,7 +242,8 @@ double testConvert(UnicodeString source, UnicodeString target, double input) { // }; // for (const auto &testCase : testCases) { -// assertEquals("test convert", testConvert(testCase.source, testCase.target, testCase.inputValue), +// assertEquals("test convert", testConvert(testCase.source, testCase.target, +// testCase.inputValue), // testCase.expectedValue); // } // } @@ -190,8 +266,8 @@ StringPiece trimField(char *(&field)[2]) { } /** - * WIP(hugovdm): deals with a single data-driven unit test for unit conversions. - * This is a UParseLineFn as required by u_parseDelimitedFile. + * Deals with a single data-driven unit test for unit conversions. This + * UParseLineFn for use by u_parseDelimitedFile is intended for "unitsTest.txt". */ void unitsTestDataLineFn(void *context, char *fields[][2], int32_t fieldCount, UErrorCode *pErrorCode) { if (U_FAILURE(*pErrorCode)) return; @@ -246,7 +322,8 @@ void unitsTestDataLineFn(void *context, char *fields[][2], int32_t fieldCount, U // unitsTest->assertTrue(msg.data(), actualState != UNCONVERTIBLE); - // Unit conversion... untested: + // TODO(hugovdm,younies): add conversion testing in unitsTestDataLineFn: + // // UnitConverter converter(sourceUnit, targetUnit, status); // double got = converter.convert(1000, status); // unitsTest->assertEqualsNear(quantity.data(), expected, got, 0.0001);