From: Travis Keep Date: Thu, 10 Oct 2013 21:52:15 +0000 (+0000) Subject: ICU-10219 Fix rounding in TimeUnitFormat. X-Git-Tag: milestone-59-0-1~2406 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=12e211c5a3ae0fcfc862bbd5fc10f14c876988bd;p=icu ICU-10219 Fix rounding in TimeUnitFormat. X-SVN-Rev: 34550 --- diff --git a/icu4c/source/i18n/tmutfmt.cpp b/icu4c/source/i18n/tmutfmt.cpp index 6dddc543723..ac1e780a750 100644 --- a/icu4c/source/i18n/tmutfmt.cpp +++ b/icu4c/source/i18n/tmutfmt.cpp @@ -11,6 +11,8 @@ #if !UCONFIG_NO_FORMATTING +#include "unicode/decimfmt.h" +#include "plurrule_impl.h" #include "uvector.h" #include "charstr.h" #include "cmemory.h" @@ -207,17 +209,32 @@ TimeUnitFormat::format(const Formattable& obj, UnicodeString& toAppendTo, const TimeUnitAmount* amount = dynamic_cast(formatObj); if (amount != NULL){ Hashtable* countToPattern = fTimeUnitToCountToPatterns[amount->getTimeUnitField()]; - double number; const Formattable& amtNumber = amount->getNumber(); - if (amtNumber.getType() == Formattable::kDouble) { - number = amtNumber.getDouble(); - } else if (amtNumber.getType() == Formattable::kLong) { - number = amtNumber.getLong(); - } else { - status = U_ILLEGAL_ARGUMENT_ERROR; + UnicodeString formattedNumber; + fNumberFormat->format(amtNumber, formattedNumber, status); + if (U_FAILURE(status)) { return toAppendTo; } - UnicodeString count = fPluralRules->select(number); + UnicodeString count; + const DecimalFormat* decfmt = dynamic_cast(fNumberFormat); + if (decfmt != NULL) { + FixedDecimal fd = decfmt->getFixedDecimal(amtNumber, status); + if (U_FAILURE(status)) { + return toAppendTo; + } + count = fPluralRules->select(fd); + } else { + if (amtNumber.getType() == Formattable::kDouble) { + count = fPluralRules->select(amtNumber.getDouble()); + } else if (amtNumber.getType() == Formattable::kLong) { + count = fPluralRules->select(amtNumber.getLong()); + } else if (amtNumber.getType() == Formattable::kInt64) { + count = fPluralRules->select((double) amtNumber.getInt64()); + } else { + status = U_ILLEGAL_ARGUMENT_ERROR; + return toAppendTo; + } + } #ifdef TMUTFMT_DEBUG char result[1000]; count.extract(0, count.length(), result, "UTF-8"); @@ -225,7 +242,7 @@ TimeUnitFormat::format(const Formattable& obj, UnicodeString& toAppendTo, #endif MessageFormat* pattern = ((MessageFormat**)countToPattern->get(count))[fStyle]; Formattable formattable[1]; - formattable[0].setDouble(number); + formattable[0].setString(formattedNumber); return pattern->format(formattable, 1, toAppendTo, pos, status); } } @@ -238,7 +255,7 @@ void TimeUnitFormat::parseObject(const UnicodeString& source, Formattable& result, ParsePosition& pos) const { - double resultNumber = -1; + Formattable resultNumber(0.0); UBool withNumberFormat = false; TimeUnit::UTimeUnitFields resultTimeUnit = TimeUnit::UTIMEUNIT_FIELD_COUNT; int32_t oldPos = pos.getIndex(); @@ -282,26 +299,21 @@ TimeUnitFormat::parseObject(const UnicodeString& source, #ifdef TMUTFMT_DEBUG std::cout << "parsed.getType: " << parsed.getType() << "\n"; #endif - double tmpNumber = 0; + Formattable tmpNumber(0.0); if (pattern->getArgTypeCount() != 0) { - // pattern with Number as beginning, such as "{0} d". - // check to make sure that the timeUnit is consistent Formattable& temp = parsed[0]; - if (temp.getType() == Formattable::kDouble) { - tmpNumber = temp.getDouble(); - } else if (temp.getType() == Formattable::kLong) { - tmpNumber = temp.getLong(); + if (temp.getType() == Formattable::kString) { + UnicodeString tmpString; + UErrorCode pStatus = U_ZERO_ERROR; + fNumberFormat->parse(temp.getString(tmpString), tmpNumber, pStatus); + if (U_FAILURE(pStatus)) { + continue; + } + } else if (temp.isNumeric()) { + tmpNumber = temp; } else { continue; } - UnicodeString select = fPluralRules->select(tmpNumber); - #ifdef TMUTFMT_DEBUG - select.extract(0, select.length(), res, "UTF-8"); - std::cout << "parse plural select count: " << res << "\n"; - #endif - if (*count != select) { - continue; - } } int32_t parseDistance = pos.getIndex() - oldPos; if (parseDistance > longestParseDistance) { @@ -327,15 +339,15 @@ TimeUnitFormat::parseObject(const UnicodeString& source, if (withNumberFormat == false && longestParseDistance != 0) { // set the number using plurrual count if (0 == countOfLongestMatch->compare(PLURAL_COUNT_ZERO, 4)) { - resultNumber = 0; + resultNumber = Formattable(0.0); } else if (0 == countOfLongestMatch->compare(PLURAL_COUNT_ONE, 3)) { - resultNumber = 1; + resultNumber = Formattable(1.0); } else if (0 == countOfLongestMatch->compare(PLURAL_COUNT_TWO, 3)) { - resultNumber = 2; + resultNumber = Formattable(2.0); } else { // should not happen. // TODO: how to handle? - resultNumber = 3; + resultNumber = Formattable(3.0); } } if (longestParseDistance == 0) { @@ -505,9 +517,6 @@ TimeUnitFormat::readFromCurrentLocale(UTimeUnitFormatStyle style, const char* ke } MessageFormat* messageFormat = new MessageFormat(pattern, fLocale, err); if ( U_SUCCESS(err) ) { - if (fNumberFormat != NULL) { - messageFormat->setFormat(0, *fNumberFormat); - } MessageFormat** formatters = (MessageFormat**)countToPatterns->get(pluralCountUniStr); if (formatters == NULL) { formatters = (MessageFormat**)uprv_malloc(UTMUTFMT_FORMAT_STYLE_COUNT*sizeof(MessageFormat*)); @@ -643,9 +652,6 @@ TimeUnitFormat::searchInLocaleChain(UTimeUnitFormatStyle style, const char* key, //found MessageFormat* messageFormat = new MessageFormat(UnicodeString(TRUE, pattern, ptLength), fLocale, err); if (U_SUCCESS(err)) { - if (fNumberFormat != NULL) { - messageFormat->setFormat(0, *fNumberFormat); - } MessageFormat** formatters = (MessageFormat**)countToPatterns->get(srcPluralCount); if (formatters == NULL) { formatters = (MessageFormat**)uprv_malloc(UTMUTFMT_FORMAT_STYLE_COUNT*sizeof(MessageFormat*)); @@ -722,9 +728,6 @@ TimeUnitFormat::searchInLocaleChain(UTimeUnitFormatStyle style, const char* key, messageFormat = new MessageFormat(UnicodeString(TRUE, pattern, -1), fLocale, err); } if (U_SUCCESS(err)) { - if (fNumberFormat != NULL && messageFormat != NULL) { - messageFormat->setFormat(0, *fNumberFormat); - } MessageFormat** formatters = (MessageFormat**)countToPatterns->get(srcPluralCount); if (formatters == NULL) { formatters = (MessageFormat**)uprv_malloc(UTMUTFMT_FORMAT_STYLE_COUNT*sizeof(MessageFormat*)); @@ -766,20 +769,6 @@ TimeUnitFormat::setNumberFormat(const NumberFormat& format, UErrorCode& status){ } delete fNumberFormat; fNumberFormat = (NumberFormat*)format.clone(); - // reset the number formatter in the fTimeUnitToCountToPatterns map - for (TimeUnit::UTimeUnitFields i = TimeUnit::UTIMEUNIT_YEAR; - i < TimeUnit::UTIMEUNIT_FIELD_COUNT; - i = (TimeUnit::UTimeUnitFields)(i+1)) { - int32_t pos = -1; - const UHashElement* elem = NULL; - while ((elem = fTimeUnitToCountToPatterns[i]->nextElement(pos)) != NULL){ - const UHashTok keyTok = elem->value; - MessageFormat** pattern = (MessageFormat**)keyTok.pointer; - - pattern[UTMUTFMT_FULL_STYLE]->setFormat(0, format); - pattern[UTMUTFMT_ABBREVIATED_STYLE]->setFormat(0, format); - } - } } diff --git a/icu4c/source/test/intltest/tufmtts.cpp b/icu4c/source/test/intltest/tufmtts.cpp index ee4339a045f..a1f4104c76e 100644 --- a/icu4c/source/test/intltest/tufmtts.cpp +++ b/icu4c/source/test/intltest/tufmtts.cpp @@ -7,6 +7,7 @@ #if !UCONFIG_NO_FORMATTING +#include "unicode/decimfmt.h" #include "unicode/tmunit.h" #include "unicode/tmutamt.h" #include "unicode/tmutfmt.h" @@ -27,10 +28,28 @@ void TimeUnitTest::runIndexedTest( int32_t index, UBool exec, const char* &name, TESTCASE(1, testAPI); TESTCASE(2, testGreekWithFallback); TESTCASE(3, testGreekWithSanitization); + TESTCASE(4, test10219Plurals); default: name = ""; break; } } +// This function is more lenient than equals operator as it considers integer 3 hours and +// double 3.0 hours to be equal +static UBool tmaEqual(const TimeUnitAmount& left, const TimeUnitAmount& right) { + if (left.getTimeUnitField() != right.getTimeUnitField()) { + return FALSE; + } + UErrorCode status = U_ZERO_ERROR; + if (!left.getNumber().isNumeric() || !right.getNumber().isNumeric()) { + return FALSE; + } + UBool result = left.getNumber().getDouble(status) == right.getNumber().getDouble(status); + if (U_FAILURE(status)) { + return FALSE; + } + return result; +} + /** * Test basic */ @@ -78,14 +97,14 @@ void TimeUnitTest::testBasic() { Formattable result; ((Format*)formats[style])->parseObject(formatted, result, status); if (!assertSuccess("parseObject()", status)) return; - if (result != formattable) { + if (!tmaEqual(*((TimeUnitAmount *)result.getObject()), *((TimeUnitAmount *) formattable.getObject()))) { dataerrln("No round trip: "); } // other style parsing Formattable result_1; ((Format*)formats[1-style])->parseObject(formatted, result_1, status); if (!assertSuccess("parseObject()", status)) return; - if (result_1 != formattable) { + if (!tmaEqual(*((TimeUnitAmount *)result_1.getObject()), *((TimeUnitAmount *) formattable.getObject()))) { dataerrln("No round trip: "); } } @@ -354,4 +373,54 @@ void TimeUnitTest::testGreekWithSanitization() { delete timeUnitFormat; } +void TimeUnitTest::test10219Plurals() { + Locale usLocale("en_US"); + UnicodeString expected[3] = {"1 minute", "1.5 minutes", "1.58 minutes"}; + UErrorCode status = U_ZERO_ERROR; + TimeUnitFormat tuf(usLocale, status); + if (U_FAILURE(status)) { + dataerrln("generating TimeUnitFormat Object failed: %s", u_errorName(status)); + return; + } + LocalPointer nf((DecimalFormat *) NumberFormat::createInstance(usLocale, status)); + if (U_FAILURE(status)) { + dataerrln("generating NumberFormat Object failed: %s", u_errorName(status)); + return; + } + for (int32_t i = 0; i < sizeof(expected) / sizeof(expected[0]); ++i) { + nf->setMaximumFractionDigits(i); + nf->setRoundingMode(DecimalFormat::kRoundDown); + tuf.setNumberFormat(*nf, status); + if (U_FAILURE(status)) { + dataerrln("setting NumberFormat failed: %s", u_errorName(status)); + return; + } + UnicodeString actual; + Formattable fmt; + LocalPointer tamt(new TimeUnitAmount(1.588, TimeUnit::UTIMEUNIT_MINUTE, status)); + if (U_FAILURE(status)) { + dataerrln("generating TimeUnitAmount Object failed: %s", u_errorName(status)); + return; + } + fmt.adoptObject(tamt.orphan()); + tuf.format(fmt, actual, status); + if (U_FAILURE(status)) { + dataerrln("Actual formatting failed: %s", u_errorName(status)); + return; + } + if (expected[i] != actual) { + errln("Expected " + expected[i] + ", got " + actual); + } + } + + // test parsing + Formattable result; + ParsePosition pos; + UnicodeString formattedString = "1 minutes"; + tuf.parseObject(formattedString, result, pos); + if (formattedString.length() != pos.getIndex()) { + errln("Expect parsing to go all the way to the end of the string."); + } +} + #endif diff --git a/icu4c/source/test/intltest/tufmtts.h b/icu4c/source/test/intltest/tufmtts.h index 57bc6ee982f..cd4a48542f1 100644 --- a/icu4c/source/test/intltest/tufmtts.h +++ b/icu4c/source/test/intltest/tufmtts.h @@ -50,6 +50,12 @@ public: */ void testGreekWithSanitization(); + /** + * Performs unit test for ticket 10219 making sure that plurals work + * correctly with rounding. + */ + void test10219Plurals(); + }; #endif /* #if !UCONFIG_NO_FORMATTING */