]> granicus.if.org Git - icu/commitdiff
ICU-10219 Fix rounding in TimeUnitFormat.
authorTravis Keep <keep94@gmail.com>
Thu, 10 Oct 2013 21:52:15 +0000 (21:52 +0000)
committerTravis Keep <keep94@gmail.com>
Thu, 10 Oct 2013 21:52:15 +0000 (21:52 +0000)
X-SVN-Rev: 34550

icu4c/source/i18n/tmutfmt.cpp
icu4c/source/test/intltest/tufmtts.cpp
icu4c/source/test/intltest/tufmtts.h

index 6dddc5437236c4179bc47554c432770f46f24efc..ac1e780a7504a392e7e95ee80d284eb1c143be51 100644 (file)
@@ -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<const TimeUnitAmount*>(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<const DecimalFormat*>(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);
-        }
-    }
 }
 
 
index ee4339a045f926f9308a23df1255b14d2856e245..a1f4104c76ef9242e0771a51dfd9624846e256ee 100644 (file)
@@ -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<DecimalFormat> 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<TimeUnitAmount> 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
index 57bc6ee982f0f03f1d39d82514017bd77019ac4f..cd4a48542f18228c207c54b2c937568f2701ebca 100644 (file)
@@ -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 */