From 2eabfa0916fae31796181ea3e101ce16e0ce6e0b Mon Sep 17 00:00:00 2001 From: Hugo van der Merwe <17109322+hugovdm@users.noreply.github.com> Date: Tue, 30 Jun 2020 19:20:26 +0200 Subject: [PATCH] Implement FormattedNumber::getOutputUnit() properly. --- icu4c/source/i18n/number_fluent.cpp | 10 ++-------- icu4c/source/i18n/number_formatimpl.cpp | 16 +++++++++------- icu4c/source/i18n/number_formatimpl.h | 8 ++++---- icu4c/source/i18n/number_output.cpp | 1 - icu4c/source/test/intltest/numbertest_api.cpp | 18 +++++++++--------- 5 files changed, 24 insertions(+), 29 deletions(-) diff --git a/icu4c/source/i18n/number_fluent.cpp b/icu4c/source/i18n/number_fluent.cpp index 4d9450730c9..579f3062142 100644 --- a/icu4c/source/i18n/number_fluent.cpp +++ b/icu4c/source/i18n/number_fluent.cpp @@ -716,15 +716,9 @@ LocalizedNumberFormatter::formatDecimalQuantity(const DecimalQuantity& dq, UErro void LocalizedNumberFormatter::formatImpl(impl::UFormattedNumberData* results, UErrorCode& status) const { if (computeCompiled(status)) { - // TODO(units,hugovdm): results needs outputUnit too, consider how to - // add that - maybe pass `results` to formatStatic() instead of just - // results->quantity and ->getStringRef()? : - fCompiled->format(results->quantity, results->getStringRef(), status); + fCompiled->format(results, status); } else { - // TODO(units,hugovdm): results needs outputUnit too, consider how to - // add that - maybe pass `results` to formatStatic() instead of just - // results->quantity and ->getStringRef()? : - NumberFormatterImpl::formatStatic(fMacros, results->quantity, results->getStringRef(), status); + NumberFormatterImpl::formatStatic(fMacros, results, status); } if (U_FAILURE(status)) { return; diff --git a/icu4c/source/i18n/number_formatimpl.cpp b/icu4c/source/i18n/number_formatimpl.cpp index 8f50de4c01d..d51db993167 100644 --- a/icu4c/source/i18n/number_formatimpl.cpp +++ b/icu4c/source/i18n/number_formatimpl.cpp @@ -32,11 +32,14 @@ NumberFormatterImpl::NumberFormatterImpl(const MacroProps& macros, UErrorCode& s : NumberFormatterImpl(macros, true, status) { } -int32_t NumberFormatterImpl::formatStatic(const MacroProps& macros, DecimalQuantity& inValue, - FormattedStringBuilder& outString, UErrorCode& status) { +int32_t NumberFormatterImpl::formatStatic(const MacroProps ¯os, UFormattedNumberData *results, + UErrorCode &status) { + DecimalQuantity &inValue = results->quantity; + FormattedStringBuilder &outString = results->getStringRef(); NumberFormatterImpl impl(macros, false, status); MicroProps& micros = impl.preProcessUnsafe(inValue, status); if (U_FAILURE(status)) { return 0; } + results->outputUnit = micros.outputUnit; int32_t length = writeNumber(micros, inValue, outString, 0, status); length += writeAffixes(micros, outString, 0, length, status); return length; @@ -54,14 +57,13 @@ int32_t NumberFormatterImpl::getPrefixSuffixStatic(const MacroProps& macros, Sig // The "unsafe" method simply re-uses fMicros, eliminating the extra copy operation. // See MicroProps::processQuantity() for details. -int32_t NumberFormatterImpl::format(DecimalQuantity &inValue, FormattedStringBuilder &outString, - UErrorCode &status) const { +int32_t NumberFormatterImpl::format(UFormattedNumberData *results, UErrorCode &status) const { + DecimalQuantity &inValue = results->quantity; + FormattedStringBuilder &outString = results->getStringRef(); MicroProps micros; preProcess(inValue, micros, status); if (U_FAILURE(status)) { return 0; } - // TODO(units,hugovdm): micros.outputUnit contains the unit we wish to - // return via FormattedNumber::getOutputUnit(). Plumb it into the pipeline - // in a reasonable way. + results->outputUnit = micros.outputUnit; int32_t length = writeNumber(micros, inValue, outString, 0, status); length += writeAffixes(micros, outString, 0, length, status); return length; diff --git a/icu4c/source/i18n/number_formatimpl.h b/icu4c/source/i18n/number_formatimpl.h index 1696f3303d7..0bb673a4ea3 100644 --- a/icu4c/source/i18n/number_formatimpl.h +++ b/icu4c/source/i18n/number_formatimpl.h @@ -16,6 +16,7 @@ #include "number_longnames.h" #include "number_compact.h" #include "number_microprops.h" +#include "number_utypes.h" U_NAMESPACE_BEGIN namespace number { namespace impl { @@ -35,9 +36,8 @@ class NumberFormatterImpl : public UMemory { /** * Builds and evaluates an "unsafe" MicroPropsGenerator, which is cheaper but can be used only once. */ - static int32_t - formatStatic(const MacroProps ¯os, DecimalQuantity &inValue, FormattedStringBuilder &outString, - UErrorCode &status); + static int32_t formatStatic(const MacroProps ¯os, UFormattedNumberData *results, + UErrorCode &status); /** * Prints only the prefix and suffix; used for DecimalFormat getters. @@ -52,7 +52,7 @@ class NumberFormatterImpl : public UMemory { /** * Evaluates the "safe" MicroPropsGenerator created by "fromMacros". */ - int32_t format(DecimalQuantity& inValue, FormattedStringBuilder& outString, UErrorCode& status) const; + int32_t format(UFormattedNumberData *results, UErrorCode &status) const; /** * Like format(), but saves the result into an output MicroProps without additional processing. diff --git a/icu4c/source/i18n/number_output.cpp b/icu4c/source/i18n/number_output.cpp index c9e54cd1ca3..f8dd200091c 100644 --- a/icu4c/source/i18n/number_output.cpp +++ b/icu4c/source/i18n/number_output.cpp @@ -35,7 +35,6 @@ void FormattedNumber::getAllFieldPositionsImpl(FieldPositionIteratorHandler& fpi fData->getAllFieldPositions(fpih, status); } -// TODO(units,hugovdm): properly implement and test this official public API. MeasureUnit FormattedNumber::getOutputUnit(UErrorCode& status) const { UPRV_FORMATTED_VALUE_METHOD_GUARD(MeasureUnit()) return fData->outputUnit; diff --git a/icu4c/source/test/intltest/numbertest_api.cpp b/icu4c/source/test/intltest/numbertest_api.cpp index 8d8bdf80300..52a04cb2955 100644 --- a/icu4c/source/test/intltest/numbertest_api.cpp +++ b/icu4c/source/test/intltest/numbertest_api.cpp @@ -683,23 +683,23 @@ void NumberFormatterApiTest::unitUsage() { LocalizedNumberFormatter formatter = unloc_formatter.locale("en-ZA"); FormattedNumber formattedNum = formatter.formatDouble(300, status); - // TODO(units,hugovdm): implement FormattedNumber::getOutputUnit() -// assertTrue("unitUsage() en-ZA road outputUnit", -// MeasureUnit::getMeter() == formattedNum.getOutputUnit(status)); + assertTrue(UnicodeString("unitUsage() en-ZA road, got outputUnit: \"") + + formattedNum.getOutputUnit(status).getIdentifier() + "\"", + MeasureUnit::getMeter() == formattedNum.getOutputUnit(status)); assertEquals("unitUsage() en-ZA road", "300 m", formattedNum.toString(status)); formatter = unloc_formatter.locale("en-GB"); formattedNum = formatter.formatDouble(300, status); - // TODO(units,hugovdm): implement FormattedNumber::getOutputUnit() -// assertTrue("unitUsage() en-GB road outputUnit", -// MeasureUnit::getYard() == formattedNum.getOutputUnit(status)); + assertTrue(UnicodeString("unitUsage() en-GB road, got outputUnit: \"") + + formattedNum.getOutputUnit(status).getIdentifier() + "\"", + MeasureUnit::getYard() == formattedNum.getOutputUnit(status)); assertEquals("unitUsage() en-GB road", "328 yd", formattedNum.toString(status)); formatter = unloc_formatter.locale("en-US"); formattedNum = formatter.formatDouble(300, status); - // TODO(units,hugovdm): implement FormattedNumber::getOutputUnit() -// assertTrue("unitUsage() en-US road outputUnit", -// MeasureUnit::getFoot() == formattedNum.getOutputUnit(status)); + assertTrue(UnicodeString("unitUsage() en-US road, got outputUnit: \"") + + formattedNum.getOutputUnit(status).getIdentifier() + "\"", + MeasureUnit::getFoot() == formattedNum.getOutputUnit(status)); assertEquals("unitUsage() en-US road", "984 ft", formattedNum.toString(status)); // TODO(hugovdm): consider fixing TODO(ICU-20941) too? -- 2.40.0