From 3b505977c0e82659896125698389b59eabe50b14 Mon Sep 17 00:00:00 2001 From: younies Date: Tue, 16 Jun 2020 18:28:30 +0200 Subject: [PATCH] fix various parts of the code --- icu4c/source/common/cmemory.h | 8 +- icu4c/source/i18n/complexunitsconverter.cpp | 36 +++-- icu4c/source/i18n/complexunitsconverter.h | 20 +-- icu4c/source/i18n/measunit_extra.cpp | 51 +++---- icu4c/source/i18n/unicode/measunit.h | 5 +- icu4c/source/i18n/unitconverter.cpp | 129 +++++++++-------- icu4c/source/i18n/unitconverter.h | 20 ++- icu4c/source/i18n/unitsrouter.cpp | 13 +- icu4c/source/test/intltest/unitstest.cpp | 149 ++++++++------------ 9 files changed, 215 insertions(+), 216 deletions(-) diff --git a/icu4c/source/common/cmemory.h b/icu4c/source/common/cmemory.h index e54d397405d..19b4f5df000 100644 --- a/icu4c/source/common/cmemory.h +++ b/icu4c/source/common/cmemory.h @@ -777,9 +777,13 @@ public: } template - void emplaceBackAndConfirm(UErrorCode &status, Args &&... args) { + T *emplaceBackAndCheckErrorCode(UErrorCode &status, Args &&... args) { T *pointer = this->create(args...); - if (pointer == nullptr) { status = U_MEMORY_ALLOCATION_ERROR; } + if (U_SUCCESS(status) && pointer == nullptr) { + status = U_MEMORY_ALLOCATION_ERROR; + } + + return pointer; } int32_t length() const { diff --git a/icu4c/source/i18n/complexunitsconverter.cpp b/icu4c/source/i18n/complexunitsconverter.cpp index 30c925ada2b..fb906cde587 100644 --- a/icu4c/source/i18n/complexunitsconverter.cpp +++ b/icu4c/source/i18n/complexunitsconverter.cpp @@ -16,12 +16,17 @@ U_NAMESPACE_BEGIN -ComplexUnitsConverter::ComplexUnitsConverter(const MeasureUnit inputUnit, const MeasureUnit outputUnits, +ComplexUnitsConverter::ComplexUnitsConverter(const MeasureUnit &inputUnit, + const MeasureUnit &outputUnits, const ConversionRates &ratesInfo, UErrorCode &status) { if (outputUnits.getComplexity(status) != UMeasureUnitComplexity::UMEASURE_UNIT_MIXED) { - unitConverters_.emplaceBackAndConfirm(status, inputUnit, outputUnits, ratesInfo, status); - units_.emplaceBackAndConfirm(status, outputUnits); + unitConverters_.emplaceBackAndCheckErrorCode(status, inputUnit, outputUnits, ratesInfo, status); + if (U_FAILURE(status)) { + return; + } + + units_.emplaceBackAndCheckErrorCode(status, outputUnits); return; } @@ -43,8 +48,9 @@ ComplexUnitsConverter::ComplexUnitsConverter(const MeasureUnit inputUnit, const auto singleUnits = outputUnits.splitToSingleUnits(length, status); MaybeStackVector singleUnitsInOrder; for (int i = 0; i < length; ++i) { - // TODO(younies): ensure units being in order in phase 2. Now, the units in order by default. - singleUnitsInOrder.emplaceBackAndConfirm(status, singleUnits[i]); + // TODO(younies): ensure units being in order by the biggest unit at first. + // This issue is part of phase 2. + singleUnitsInOrder.emplaceBackAndCheckErrorCode(status, singleUnits[i]); } if (singleUnitsInOrder.length() == 0) { @@ -54,14 +60,16 @@ ComplexUnitsConverter::ComplexUnitsConverter(const MeasureUnit inputUnit, const for (int i = 0, n = singleUnitsInOrder.length(); i < n; i++) { if (i == 0) { // first element - unitConverters_.emplaceBackAndConfirm(status, inputUnit, *singleUnitsInOrder[i], ratesInfo, - status); + unitConverters_.emplaceBackAndCheckErrorCode(status, inputUnit, *singleUnitsInOrder[i], + ratesInfo, status); } else { - unitConverters_.emplaceBackAndConfirm(status, *singleUnitsInOrder[i - 1], - *singleUnitsInOrder[i], ratesInfo, status); + unitConverters_.emplaceBackAndCheckErrorCode(status, *singleUnitsInOrder[i - 1], + *singleUnitsInOrder[i], ratesInfo, status); } - if (U_FAILURE(status)) { return; } + if (U_FAILURE(status)) { + return; + } } units_.appendAll(singleUnitsInOrder, status); @@ -85,8 +93,8 @@ MaybeStackVector ComplexUnitsConverter::convert(double quantity, UError Formattable formattableNewQuantity(newQuantity); // NOTE: Measure would own its MeasureUnit. - result.emplaceBackAndConfirm(status, formattableNewQuantity, new MeasureUnit(*units_[i]), - status); + result.emplaceBackAndCheckErrorCode(status, formattableNewQuantity, + new MeasureUnit(*units_[i]), status); // Keep the residual of the quantity. // For example: `3.6 feet`, keep only `0.6 feet` @@ -95,8 +103,8 @@ MaybeStackVector ComplexUnitsConverter::convert(double quantity, UError Formattable formattableQuantity(quantity); // NOTE: Measure would own its MeasureUnit. - result.emplaceBackAndConfirm(status, formattableQuantity, new MeasureUnit(*units_[i]), - status); + result.emplaceBackAndCheckErrorCode(status, formattableQuantity, new MeasureUnit(*units_[i]), + status); } } diff --git a/icu4c/source/i18n/complexunitsconverter.h b/icu4c/source/i18n/complexunitsconverter.h index 9dce941fe59..933107bd332 100644 --- a/icu4c/source/i18n/complexunitsconverter.h +++ b/icu4c/source/i18n/complexunitsconverter.h @@ -17,7 +17,8 @@ U_NAMESPACE_BEGIN /** - * Convert from single unit to multiple/complex unit. For example, from `meter` to `foot+inch`. + * Converts from single or compound unit to single, compound or mixed units. + * For example, from `meter` to `foot+inch`. * * DESIGN: * This class uses `UnitConverter` in order to perform the single converter (i.e. converters from a @@ -31,26 +32,27 @@ class U_I18N_API ComplexUnitsConverter { * NOTE: * - inputUnit and outputUnits must be under the same category * - e.g. meter to feet and inches --> all of them are length units. - * @param inputUnit represents the source unit. (should be single unit) - * @param outputUnits a single unit or multi units. For example (`inch` or `foot+inch`) - * @param lengthOfOutputUnits represents the length of the output units. + * + * @param inputUnit represents the source unit. (should be single or compound unit). + * @param outputUnits represents the output unit. could be any type. (single, compound or mixed). * @param status */ - ComplexUnitsConverter(const MeasureUnit inputUnit, const MeasureUnit outputUnits, + ComplexUnitsConverter(const MeasureUnit &inputUnit, const MeasureUnit &outputUnits, const ConversionRates &ratesInfo, UErrorCode &status); - // Returns true if the `quantity` in the `inputUnit` is greater than or equal than the `limit` in the - // biggest `outputUnits` + // Returns true if the specified `quantity` of the `inputUnit`, expressed in terms of the biggest + // unit in the MeasureUnit `outputUnit`, is greater than or equal to `limit`. // For example, if the input unit is `meter` and the target unit is `foot+inch`. Therefore, this // function will convert the `quantity` from `meter` to `foot`, then, it will compare the value in // `foot` with the `limit`. UBool greaterThanOrEqual(double quantity, double limit) const; - // Returns outputMeasures which is an array with the correspinding values. + // Returns outputMeasures which is an array with the corresponding values. // - E.g. converting meters to feet and inches. // 1 meter --> 3 feet, 3.3701 inches // NOTE: - // the smallest element is the only element that could has fractional values. + // the smallest element is the only element that could have fractional values. And all + // other elements are rounded to the nearest integer MaybeStackVector convert(double quantity, UErrorCode &status) const; private: diff --git a/icu4c/source/i18n/measunit_extra.cpp b/icu4c/source/i18n/measunit_extra.cpp index 982a095d433..3f1891ac75a 100644 --- a/icu4c/source/i18n/measunit_extra.cpp +++ b/icu4c/source/i18n/measunit_extra.cpp @@ -770,55 +770,40 @@ MeasureUnit MeasureUnit::product(const MeasureUnit& other, UErrorCode& status) c } /** - * Searches the `simplifiedUnits` for a unit with the same base identifier in the `newUnit`, for example - * `meter` and `meter` or `millimeter` and `centimeter`. - * After finding a match, the matched unit in simplified unit will be merged with the `newUnit` and - * `true` will be returned. Otherwise, `false` will be returned. + * Searches the `simplifiedUnits` for a unit with the same base identifier and the same SI prefix. + * For example, `square-meter` and `cubic-meter` but not `meter` and `centimeter`. + * After that, the matched units will be merged. */ -bool findAndSet(MaybeStackVector &simplifiedUnits, const MeasureUnit &newUnit, - UErrorCode &status) { - for (int i = 0, n = simplifiedUnits.length(); i < n; i++) { - auto simplifiedUnitImpl = SingleUnitImpl::forMeasureUnit(*simplifiedUnits[i], status); - auto newUnitImpl = SingleUnitImpl::forMeasureUnit(newUnit, status); - if (simplifiedUnitImpl.identifier == newUnitImpl.identifier) { - int32_t newDimensionality = simplifiedUnitImpl.dimensionality + newUnitImpl.dimensionality; - UMeasureSIPrefix newSIprefix = static_cast( - simplifiedUnitImpl.siPrefix * simplifiedUnitImpl.dimensionality + - newUnitImpl.siPrefix * newUnitImpl.dimensionality); +void findAndMerge(MeasureUnitImpl &simplifiedUnitsImpl, const SingleUnitImpl &newUnitImpl, + UErrorCode &status) { + for (int i = 0, n = simplifiedUnitsImpl.units.length(); i < n; i++) { + auto& singleSimplifiedUnitImpl = *(simplifiedUnitsImpl.units[i]); + if (singleSimplifiedUnitImpl.identifier == newUnitImpl.identifier && + singleSimplifiedUnitImpl.siPrefix == newUnitImpl.siPrefix) { - auto &simplifiedUnit = *simplifiedUnits[i]; - simplifiedUnit = simplifiedUnit.withDimensionality(newDimensionality, status); - simplifiedUnit = simplifiedUnit.withSIPrefix(newSIprefix ,status); - - return true; + singleSimplifiedUnitImpl.dimensionality += newUnitImpl.dimensionality; + return; } } - return false; + simplifiedUnitsImpl.units.emplaceBackAndCheckErrorCode(status, newUnitImpl); } MeasureUnit MeasureUnit::simplify(UErrorCode &status) const { - MeasureUnit result; if (this->getComplexity(status) == UMeasureUnitComplexity::UMEASURE_UNIT_MIXED) { status = U_INTERNAL_PROGRAM_ERROR; - return result; + return MeasureUnit(); } + MeasureUnitImpl resultImpl; + int32_t outCount; auto singleUnits = this->splitToSingleUnits(outCount, status); - - MaybeStackVector simplifiedUnits; - for (int i = 0 ; i < outCount ; ++i) { - if (findAndSet(simplifiedUnits,singleUnits[i] , status )) { continue;} - - simplifiedUnits.emplaceBackAndConfirm(status, singleUnits[i]); + for (int i = 0; i < outCount; ++i) { + findAndMerge(resultImpl, SingleUnitImpl::forMeasureUnit(singleUnits[i], status), status); } - for (int i = 0 , n = simplifiedUnits.length() ; i < n ; ++i) { - result = result.product(*simplifiedUnits[i] , status); - } - - return result; + return std::move(resultImpl).build(status); } LocalArray MeasureUnit::splitToSingleUnits(int32_t& outCount, UErrorCode& status) const { diff --git a/icu4c/source/i18n/unicode/measunit.h b/icu4c/source/i18n/unicode/measunit.h index d857d8aeb79..e37854ee2b7 100644 --- a/icu4c/source/i18n/unicode/measunit.h +++ b/icu4c/source/i18n/unicode/measunit.h @@ -447,9 +447,12 @@ class U_I18N_API MeasureUnit: public UObject { * NOTE: Only works on SINGLE and COMPOUND units. If the current unit is a * MIXED unit, an error will occur. For more information, see UMeasureUnitComplexity. * + * NOTE: Only merge units that are having the same identifier and the same SI prefix. + * For example, `square-meter` and `cubic-meter`, but not `meter` and `centimeter`. + * * @param status Set if this is a MIXED unit or if another error occurs. * @return a simplified version of the current unit. - * @draft ICU 67 + * @draft ICU 68 */ MeasureUnit simplify( UErrorCode& status) const; diff --git a/icu4c/source/i18n/unitconverter.cpp b/icu4c/source/i18n/unitconverter.cpp index ac3a4f86c4c..f73113ab9c5 100644 --- a/icu4c/source/i18n/unitconverter.cpp +++ b/icu4c/source/i18n/unitconverter.cpp @@ -126,7 +126,9 @@ struct Factor { constantsValues[CONSTANT_GAL_IMP2M3] = 0.00454609; for (int i = 0; i < CONSTANTS_COUNT; i++) { - if (this->constants[i] == 0) { continue; } + if (this->constants[i] == 0) { + continue; + } auto absPower = std::abs(this->constants[i]); SigNum powerSig = this->constants[i] < 0 ? SigNum::NEGATIVE : SigNum::POSITIVE; @@ -155,7 +157,9 @@ double strToDouble(StringPiece strNum, UErrorCode &status) { StringToDoubleConverter converter(0, 0, 0, "", ""); int32_t count; double result = converter.StringToDouble(strNum.data(), strNum.length(), &count); - if (count != strNum.length()) { status = U_INVALID_FORMAT_ERROR; } + if (count != strNum.length()) { + status = U_INVALID_FORMAT_ERROR; + } return result; } @@ -179,56 +183,6 @@ double strHasDivideSignToDouble(StringPiece strWithDivide, UErrorCode &status) { return strToDouble(strWithDivide, status); } -} // namespace -// FIXME/TODO: this namespace closing and reopening hack makes below -// function "public" with easy-to-read diffs. But better to move it to other -// public functions. - -/** - * 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.identifier, 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; -} - -// FIXME/TODO: fix this before merging the PR! -namespace { - -// TODO: Load those constant from units data. /* * Adds a single factor element to the `Factor`. e.g "ft3m", "2.333" or "cup2m3". But not "cup2m3^3". */ @@ -301,8 +255,6 @@ void addFactorElement(Factor &factor, StringPiece elementStr, SigNum sigNum, UEr /* * Extracts `Factor` from a complete string factor. e.g. "ft2m^3*1007/cup2m3*3" - * - * TODO: warning: unused parameter 'status' [-Wunused-parameter] */ Factor extractFactorConversions(StringPiece stringFactor, UErrorCode &status) { Factor result; @@ -378,7 +330,9 @@ UBool checkSimpleUnit(const MeasureUnit &unit, UErrorCode &status) { const auto &compoundSourceUnit = MeasureUnitImpl::forMeasureUnit(unit, memory, status); if (U_FAILURE(status)) return false; - if (compoundSourceUnit.complexity != UMEASURE_UNIT_SINGLE) { return false; } + if (compoundSourceUnit.complexity != UMEASURE_UNIT_SINGLE) { + return false; + } U_ASSERT(compoundSourceUnit.units.length() == 1); auto singleUnit = *(compoundSourceUnit.units[0]); @@ -432,10 +386,62 @@ void loadConversionRate(ConversionRate &conversionRate, const MeasureUnit &sourc } // 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 U_I18N_API 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.identifier, 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; +} + UnitsConvertibilityState U_I18N_API checkConvertibility(const MeasureUnit &source, const MeasureUnit &target, const ConversionRates &conversionRates, UErrorCode &status) { + if (source.getComplexity(status) == UMeasureUnitComplexity::UMEASURE_UNIT_MIXED || + target.getComplexity(status) == UMeasureUnitComplexity::UMEASURE_UNIT_MIXED) { + status = U_INTERNAL_PROGRAM_ERROR; + return UNCONVERTIBLE; + } + auto sourceBaseUnit = extractCompoundBaseUnit(source, conversionRates, status); auto targetBaseUnit = extractCompoundBaseUnit(target, conversionRates, status); @@ -455,6 +461,13 @@ UnitsConvertibilityState U_I18N_API checkConvertibility(const MeasureUnit &sourc UnitConverter::UnitConverter(MeasureUnit source, MeasureUnit target, const ConversionRates &ratesInfo, UErrorCode &status) { + + if (source.getComplexity(status) == UMeasureUnitComplexity::UMEASURE_UNIT_MIXED || + target.getComplexity(status) == UMeasureUnitComplexity::UMEASURE_UNIT_MIXED) { + status = U_INTERNAL_PROGRAM_ERROR; + return; + } + UnitsConvertibilityState unitsState = checkConvertibility(source, target, ratesInfo, status); if (U_FAILURE(status)) return; if (unitsState == UnitsConvertibilityState::UNCONVERTIBLE) { @@ -478,11 +491,13 @@ double UnitConverter::convert(double inputValue) const { if (result == 0) return 0.0; // If the result is zero, it does not matter if the conversion are reciprocal or not. - if (conversionRate_.reciprocal) { result = 1.0 / result; } + if (conversionRate_.reciprocal) { + result = 1.0 / result; + } // Multiply the result by 1.000,000,001 to fix the deterioration from using `double` // TODO: remove the multiplication by 1.000,000,001 after using `decNumber` - return result * 1.000000001; + return result * 1.000000000001; } U_NAMESPACE_END diff --git a/icu4c/source/i18n/unitconverter.h b/icu4c/source/i18n/unitconverter.h index 61ed7f4e688..9651e0c6312 100644 --- a/icu4c/source/i18n/unitconverter.h +++ b/icu4c/source/i18n/unitconverter.h @@ -34,9 +34,21 @@ enum U_I18N_API UnitsConvertibilityState { UNCONVERTIBLE, }; -MeasureUnit extractCompoundBaseUnit(const MeasureUnit &source, const ConversionRates &conversionRates, - UErrorCode &status); +MeasureUnit U_I18N_API extractCompoundBaseUnit(const MeasureUnit &source, + const ConversionRates &conversionRates, + UErrorCode &status); +/** + * Check if the convertibility between `source` and `target`. + * For example: + * `meter` and `foot` are `CONVERTIBLE`. + * `meter-per-second` and `second-per-meter` are `RECIPROCAL`. + * `meter` and `pound` are `UNCONVERTIBLE`. + * + * NOTE: + * Only works with SINGLE and COMPOUND units. If one of the units is a + * MIXED unit, an error will occur. For more information, see UMeasureUnitComplexity. + */ UnitsConvertibilityState U_I18N_API checkConvertibility(const MeasureUnit &source, const MeasureUnit &target, const ConversionRates &conversionRates, @@ -46,8 +58,8 @@ UnitsConvertibilityState U_I18N_API checkConvertibility(const MeasureUnit &sourc * Converts from a source `MeasureUnit` to a target `MeasureUnit`. * * NOTE: - * the source and the target must be singular such as `meter` to `mile` or `mile-per-second` to - * `kilometer-per-millisecond`. However, `foot+inch` is not permitted. + * Only works with SINGLE and COMPOUND units. If one of the units is a + * MIXED unit, an error will occur. For more information, see UMeasureUnitComplexity. */ class U_I18N_API UnitConverter : public UMemory { public: diff --git a/icu4c/source/i18n/unitsrouter.cpp b/icu4c/source/i18n/unitsrouter.cpp index ca5f8cbf724..9f787ce82e6 100644 --- a/icu4c/source/i18n/unitsrouter.cpp +++ b/icu4c/source/i18n/unitsrouter.cpp @@ -29,7 +29,6 @@ UnitsRouter::UnitsRouter(MeasureUnit inputUnit, StringPiece region, StringPiece MeasureUnit baseUnit = extractCompoundBaseUnit(inputUnit, conversionRates, status); CharString category = getUnitCategory(baseUnit.getIdentifier(), status); - // TODO: deal correctly with StringPiece / null-terminated string incompatibility... const UnitPreference *const *unitPreferences; int32_t preferencesCount; prefs.getPreferencesFor(category.data(), usage, region, unitPreferences, preferencesCount, status); @@ -38,11 +37,15 @@ 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; } + if (U_FAILURE(status)) { + return; + } - converterPreferences_.emplaceBackAndConfirm(status, inputUnit, complexTargetUnit, preference.geq, - conversionRates, status); - if (U_FAILURE(status)) { return; } + converterPreferences_.emplaceBackAndCheckErrorCode(status, inputUnit, complexTargetUnit, + preference.geq, conversionRates, status); + if (U_FAILURE(status)) { + return; + } } } diff --git a/icu4c/source/test/intltest/unitstest.cpp b/icu4c/source/test/intltest/unitstest.cpp index d2434c75864..ded263e0d7c 100644 --- a/icu4c/source/test/intltest/unitstest.cpp +++ b/icu4c/source/test/intltest/unitstest.cpp @@ -5,8 +5,8 @@ #if !UCONFIG_NO_FORMATTING +#include #include -#include #include "charstr.h" #include "cmemory.h" @@ -40,45 +40,29 @@ class UnitsTest : public IntlTest { void testConversionCapability(); void testConversions(); void testPreferences(); - - void testBasic(); void testSiPrefixes(); void testMass(); void testTemperature(); void testArea(); - void testStatus(); - - // TODO(younies): remove after using CLDR test cases. - void verifyTestCase(const UnitConversionTestCase &testCase); }; 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); TESTCASE_AUTO(testSiPrefixes); TESTCASE_AUTO(testMass); TESTCASE_AUTO(testTemperature); TESTCASE_AUTO(testArea); - TESTCASE_AUTO(testStatus); TESTCASE_AUTO_END; } -// 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"kilometer" && target == u"foot" && input == 1.0) return 328.084; - - return -1; -} - void UnitsTest::testConversionCapability() { struct TestCase { const StringPiece source; @@ -108,47 +92,6 @@ void UnitsTest::testConversionCapability() { } } -void UnitsTest::verifyTestCase(const UnitConversionTestCase &testCase) { - UErrorCode status = U_ZERO_ERROR; - MeasureUnit sourceUnit = MeasureUnit::forIdentifier(testCase.source, status); - MeasureUnit targetUnit = MeasureUnit::forIdentifier(testCase.target, status); - - ConversionRates conversionRates(status); - UnitConverter converter(sourceUnit, targetUnit, conversionRates, status); - - double actual = converter.convert(testCase.inputValue); - - assertEqualsNear("test Conversion", testCase.expectedValue, actual, 0.0001); -} - -void UnitsTest::testBasic() { - IcuTestErrorCode status(*this, "Units testBasic"); - - // Test Cases - struct TestCase { - StringPiece source; - StringPiece target; - const double inputValue; - const double expectedValue; - } testCases[]{ - {"meter", "foot", 1.0, 3.28084}, // - {"kilometer", "foot", 1.0, 3280.84}, // - }; - - 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); - UnitConverter converter(source, target, conversionRates, status); - - assertEqualsNear("test conversion", testCase.expectedValue, - converter.convert(testCase.inputValue), 0.001); - } -} - void UnitsTest::testSiPrefixes() { IcuTestErrorCode status(*this, "Units testSiPrefixes"); // Test Cases @@ -328,9 +271,11 @@ struct UnitsTestContext { * @param pErrorCode Receives status. */ void unitsTestDataLineFn(void *context, char *fields[][2], int32_t fieldCount, UErrorCode *pErrorCode) { - if (U_FAILURE(*pErrorCode)) { return; } + if (U_FAILURE(*pErrorCode)) { + return; + } UnitsTestContext *ctx = (UnitsTestContext *)context; - UnitsTest* unitsTest = ctx->unitsTest; + UnitsTest *unitsTest = ctx->unitsTest; (void)fieldCount; // unused UParseLineFn variable IcuTestErrorCode status(*unitsTest, "unitsTestDatalineFn"); @@ -341,20 +286,25 @@ void unitsTestDataLineFn(void *context, char *fields[][2], int32_t fieldCount, U StringPiece utf8Expected = trimField(fields[4]); UNumberFormat *nf = unum_open(UNUM_DEFAULT, NULL, -1, "en_US", NULL, status); - if (status.errIfFailureAndReset("unum_open failed")) { return; } + if (status.errIfFailureAndReset("unum_open failed")) { + return; + } UnicodeString uExpected = UnicodeString::fromUTF8(utf8Expected); double expected = unum_parseDouble(nf, uExpected.getBuffer(), uExpected.length(), 0, status); unum_close(nf); - if (status.errIfFailureAndReset("unum_parseDouble(\"%.*s\") failed", uExpected.length(), - uExpected.getBuffer())) { + if (status.errIfFailureAndReset("unum_parseDouble(\"%s\") failed", utf8Expected)) { return; } MeasureUnit sourceUnit = MeasureUnit::forIdentifier(x, status); - if (status.errIfFailureAndReset("forIdentifier(\"%.*s\")", x.length(), x.data())) { return; } + if (status.errIfFailureAndReset("forIdentifier(\"%.*s\")", x.length(), x.data())) { + return; + } MeasureUnit targetUnit = MeasureUnit::forIdentifier(y, status); - if (status.errIfFailureAndReset("forIdentifier(\"%.*s\")", y.length(), y.data())) { return; } + if (status.errIfFailureAndReset("forIdentifier(\"%.*s\")", y.length(), y.data())) { + return; + } unitsTest->logln("Quantity (Category): \"%.*s\", " "Expected value of \"1000 %.*s in %.*s\": %f, " @@ -373,7 +323,9 @@ void unitsTestDataLineFn(void *context, char *fields[][2], int32_t fieldCount, U .append(sourceUnit.getIdentifier(), status) .append(" -> ", status) .append(targetUnit.getIdentifier(), status); - if (status.errIfFailureAndReset("msg construction")) { return; } + if (status.errIfFailureAndReset("msg construction")) { + return; + } unitsTest->assertNotEquals(msg.data(), UNCONVERTIBLE, convertibility); // Conversion: @@ -477,7 +429,7 @@ class ExpectedOutput { _skippedFields++; if (_skippedFields < 2) { // We are happy skipping one field per output unit: we want to skip - // rational fraction files like "11 / 10". + // rational fraction fields like "11 / 10". errorCode = U_ZERO_ERROR; return; } else { @@ -501,8 +453,9 @@ class ExpectedOutput { } }; +// TODO(Hugo): Add a comment and Use AssertEqualsNear. void checkOutput(UnitsTest *unitsTest, const char *msg, ExpectedOutput expected, - const MaybeStackVector &actual, double precision) { + const MaybeStackVector &actual, double precision) { IcuTestErrorCode status(*unitsTest, "checkOutput"); bool success = true; if (expected._compoundCount != actual.length()) { @@ -513,13 +466,14 @@ void checkOutput(UnitsTest *unitsTest, const char *msg, ExpectedOutput expected, break; } - double diff = std::abs(expected._amounts[i] - - actual[i]->getNumber().getDouble(status)); - double diffPercent = - expected._amounts[i] != 0 ? diff / expected._amounts[i] : diff; + //assertEqualsNear("test conversion", expected._amounts[i], + // actual[i]->getNumber().getDouble(status), 0.0001); + + 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; + success = false; + break; } if (expected._measureUnits[i] != actual[i]->getUnit()) { @@ -594,7 +548,9 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie region.data(), inputAmount, inputMeasureUnit.getIdentifier(), expected.toDebugString().c_str()); - if (U_FAILURE(status)) { return; } + if (U_FAILURE(status)) { + return; + } UnitsRouter router(inputMeasureUnit, region, usage, status); if (status.errIfFailureAndReset("UnitsRouter(<%s>, \"%.*s\", \"%.*s\", status)", inputMeasureUnit.getIdentifier(), region.length(), region.data(), @@ -611,9 +567,13 @@ void unitPreferencesTestDataLineFn(void *context, char *fields[][2], int32_t fie msg.append(inputD, status); msg.append(" ", status); msg.append(inputMeasureUnit.getIdentifier(), status); - if (status.errIfFailureAndReset("Failure before router.route")) { return; } + if (status.errIfFailureAndReset("Failure before router.route")) { + return; + } MaybeStackVector result = router.route(inputAmount, status); - if (status.errIfFailureAndReset("router.route(inputAmount, ...)")) { return; } + if (status.errIfFailureAndReset("router.route(inputAmount, ...)")) { + return; + } checkOutput(unitsTest, msg.data(), expected, result, 0.0001); } @@ -633,7 +593,9 @@ void parsePreferencesTests(const char *filename, char delimiter, char *fields[][ char *start, *limit; int32_t i; - if (U_FAILURE(*pErrorCode)) { return; } + if (U_FAILURE(*pErrorCode)) { + return; + } if (fields == NULL || lineFn == NULL || maxFieldCount <= 0) { *pErrorCode = U_ILLEGAL_ARGUMENT_ERROR; @@ -659,7 +621,9 @@ void parsePreferencesTests(const char *filename, char delimiter, char *fields[][ *pErrorCode = U_ZERO_ERROR; /* skip this line if it is empty or a comment */ - if (*start == 0 || *start == '#') { continue; } + if (*start == 0 || *start == '#') { + continue; + } /* remove in-line comments */ limit = uprv_strchr(start, '#'); @@ -674,7 +638,9 @@ void parsePreferencesTests(const char *filename, char delimiter, char *fields[][ } /* skip lines with only whitespace */ - if (u_skipWhitespace(start)[0] == 0) { continue; } + if (u_skipWhitespace(start)[0] == 0) { + continue; + } /* for each field, call the corresponding field function */ for (i = 0; i < maxFieldCount; ++i) { @@ -696,15 +662,21 @@ void parsePreferencesTests(const char *filename, char delimiter, char *fields[][ break; } } - if (i == maxFieldCount) { *pErrorCode = U_PARSE_ERROR; } + if (i == maxFieldCount) { + *pErrorCode = U_PARSE_ERROR; + } int fieldCount = i + 1; /* call the field function */ lineFn(context, fields, fieldCount, pErrorCode); - if (U_FAILURE(*pErrorCode)) { break; } + if (U_FAILURE(*pErrorCode)) { + break; + } } - if (filename != NULL) { T_FileStream_close(file); } + if (filename != NULL) { + T_FileStream_close(file); + } } /** @@ -733,9 +705,4 @@ void UnitsTest::testPreferences() { } } -/** - * Tests different return statuses depending on the input. - */ -void UnitsTest::testStatus() {} - #endif /* #if !UCONFIG_NO_FORMATTING */