ICU-13135 Fixing and optimizing PluralFormat call site into NumberFormat in order...

X-SVN-Rev: 41237

diff --git a/icu4c/source/i18n/fmtable.cpp b/icu4c/source/i18n/fmtable.cpp
index dd465a60e0fd4ac4a1044d2ef06646c6dfc75a15..f1b98a36441a85dd5d8b98721f244f743acbb64a 100644 (file)
--- a/icu4c/source/i18n/fmtable.cpp
+++ b/icu4c/source/i18n/fmtable.cpp
@@ -716,28 +716,11 @@ CharString *Formattable::internalGetCharString(UErrorCode &status) {
         // from parsing, or from the user setting a decimal number, fDecimalNum
         // would already be set.
         //
-        fDecimalQuantity = new DecimalQuantity(); // TODO: use internal digit list
-        if (fDecimalQuantity == NULL) {
-          status = U_MEMORY_ALLOCATION_ERROR;
-          return NULL;
-        }
-
-        switch (fType) {
-        case kDouble:
-          fDecimalQuantity->setToDouble(this->getDouble());
-          fDecimalQuantity->roundToInfinity();
-          break;
-        case kLong:
-          fDecimalQuantity->setToInt(this->getLong());
-          break;
-        case kInt64:
-          fDecimalQuantity->setToLong(this->getInt64());
-          break;
-        default:
-          // The formattable's value is not a numeric type.
-          status = U_INVALID_STATE_ERROR;
-          return NULL;
-        }
+        LocalPointer<DecimalQuantity> dq(new DecimalQuantity(), status);
+        if (U_FAILURE(status)) { return nullptr; }
+        populateDecimalQuantity(*dq, status);
+        if (U_FAILURE(status)) { return nullptr; }
+        fDecimalQuantity = dq.orphan();
       }
 
       fDecimalStr = new CharString();
@@ -759,6 +742,30 @@ CharString *Formattable::internalGetCharString(UErrorCode &status) {
     return fDecimalStr;
 }
 
+void
+Formattable::populateDecimalQuantity(number::impl::DecimalQuantity& output, UErrorCode& status) const {
+    if (fDecimalQuantity != nullptr) {
+        output = *fDecimalQuantity;
+        return;
+    }
+
+    switch (fType) {
+        case kDouble:
+            output.setToDouble(this->getDouble());
+            output.roundToInfinity();
+            break;
+        case kLong:
+            output.setToInt(this->getLong());
+            break;
+        case kInt64:
+            output.setToLong(this->getInt64());
+            break;
+        default:
+            // The formattable's value is not a numeric type.
+            status = U_INVALID_STATE_ERROR;
+    }
+}
+
 // ---------------------------------------
 void
 Formattable::adoptDecimalQuantity(DecimalQuantity *dq) {

diff --git a/icu4c/source/i18n/plurfmt.cpp b/icu4c/source/i18n/plurfmt.cpp
index eb8b5d88a8fa721d962198a47f16f8e2ac666b47..2775766d32df801f1f3304d80ff6ee3bae44b9a8 100644 (file)
--- a/icu4c/source/i18n/plurfmt.cpp
+++ b/icu4c/source/i18n/plurfmt.cpp
@@ -22,6 +22,8 @@
 #include "uassert.h"
 #include "uhash.h"
 #include "number_decimalquantity.h"
+#include "number_utils.h"
+#include "number_utypes.h"
 
 #if !UCONFIG_NO_FORMATTING
 
@@ -259,34 +261,33 @@ PluralFormat::format(const Formattable& numberObject, double number,
     if (msgPattern.countParts() == 0) {
         return numberFormat->format(numberObject, appendTo, pos, status);
     }
+
     // Get the appropriate sub-message.
     // Select it based on the formatted number-offset.
     double numberMinusOffset = number - offset;
-    UnicodeString numberString;
-    DecimalQuantity quantity;
-    FieldPosition ignorePos;
+    // Call NumberFormatter to get both the DecimalQuantity and the string.
+    // This call site needs to use more internal APIs than the Java equivalent.
+    number::impl::UFormattedNumberData data;
     if (offset == 0) {
-        DecimalFormat *decFmt = dynamic_cast<DecimalFormat *>(numberFormat);
-        if(decFmt != NULL) {
-            decFmt->formatToDecimalQuantity(numberObject, quantity, status);
-        } else {
-            numberFormat->format(
-                    numberObject, numberString, ignorePos, status);  // could be BigDecimal etc.
-            quantity.setToDouble(numberMinusOffset);
-            quantity.roundToInfinity();
-        }
+        // could be BigDecimal etc.
+        numberObject.populateDecimalQuantity(data.quantity, status);
     } else {
-        DecimalFormat *decFmt = dynamic_cast<DecimalFormat *>(numberFormat);
-        if(decFmt != NULL) {
-            decFmt->formatToDecimalQuantity(numberMinusOffset, quantity, status);
+        data.quantity.setToDouble(numberMinusOffset);
+    }
+    UnicodeString numberString;
+    auto *decFmt = dynamic_cast<DecimalFormat *>(numberFormat);
+    if(decFmt != nullptr) {
+        decFmt->toNumberFormatter().formatImpl(&data, status); // mutates &data
+        numberString = data.string.toUnicodeString();
+    } else {
+        if (offset == 0) {
+            numberFormat->format(numberObject, numberString, status);
         } else {
-            numberFormat->format(
-                    numberMinusOffset, numberString, ignorePos, status);
-            quantity.setToDouble(numberMinusOffset);
-            quantity.roundToInfinity();
+            numberFormat->format(numberMinusOffset, numberString, status);
         }
     }
-    int32_t partIndex = findSubMessage(msgPattern, 0, pluralRulesWrapper, &quantity, number, status);
+
+    int32_t partIndex = findSubMessage(msgPattern, 0, pluralRulesWrapper, &data.quantity, number, status);
     if (U_FAILURE(status)) { return appendTo; }
     // Replace syntactic # signs in the top level of this sub-message
     // (not in nested arguments) with the formatted number-offset.

diff --git a/icu4c/source/i18n/unicode/fmtable.h b/icu4c/source/i18n/unicode/fmtable.h
index 9f729c918a4056dd706db03891531830990b2fe4..e27cb9e808741877a7e38e8d15f54f494615c382 100644 (file)
--- a/icu4c/source/i18n/unicode/fmtable.h
+++ b/icu4c/source/i18n/unicode/fmtable.h
@@ -659,6 +659,12 @@ public:
      */
     number::impl::DecimalQuantity *getDecimalQuantity() const { return fDecimalQuantity;}
 
+    /**
+     * Export the value of this Formattable to a DecimalQuantity.
+     * @internal
+     */
+    void populateDecimalQuantity(number::impl::DecimalQuantity& output, UErrorCode& status) const;
+
     /**
      *  Adopt, and set value from, a DecimalQuantity
      *     Internal Function, do not use.

diff --git a/icu4c/source/test/intltest/plurfmts.cpp b/icu4c/source/test/intltest/plurfmts.cpp
index db2485d31f73b74609fd107a215e4cc7a015838d..c9f49f74c8aeb2e052b57730428996a55b6a75b0 100644 (file)
--- a/icu4c/source/test/intltest/plurfmts.cpp
+++ b/icu4c/source/test/intltest/plurfmts.cpp
@@ -55,7 +55,7 @@ void PluralFormatTest::pluralFormatBasicTest(/*char *par*/)
     PluralFormat* plFmt[8];
     Locale        locale = Locale::getDefault();
     UnicodeString otherPattern = UnicodeString("other{#}");
-    UnicodeString message=UnicodeString("ERROR: PluralFormat basic test");
+    UnicodeString message=UnicodeString("PluralFormat basic test");
 
     // ========= Test constructors
     logln(" Testing PluralFormat constructors ...");
@@ -685,7 +685,7 @@ PluralFormatTest::TestDecimals() {
 }
 
 void
-PluralFormatTest::numberFormatTest(PluralFormat* plFmt, 
+PluralFormatTest::numberFormatTest(PluralFormat* plFmt,
                                    NumberFormat *numFmt,
                                    int32_t start,
                                    int32_t end,
@@ -723,12 +723,15 @@ PluralFormatTest::numberFormatTest(PluralFormat* plFmt,
                 }
             }
         }
-        if ( (numResult!=plResult) || U_FAILURE(status) ) {
+        if (U_FAILURE(status)) {
+            assertSuccess(*message + " in numberFormatTest", status);
+        }
+        if (numResult!=plResult) {
             if ( message == NULL ) {
                 errln("ERROR: Unexpected plural format - got:"+plResult+ UnicodeString("  expecting:")+numResult);
             }
             else {
-                errln( *message+UnicodeString("  got:")+plResult+UnicodeString("  expecting:")+numResult);
+                assertEquals(*message + " in numberFormatTest", numResult, plResult);
             }
         }
     }

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java
index 8d8e791ccaa6d8e35fdfcdd7219dc7609a1babe1..f298d3f6053950355307e7e964296839cc89f502 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/impl/number/DecimalQuantity_DualStorageBCD.java
@@ -60,6 +60,8 @@ public final class DecimalQuantity_DualStorageBCD extends DecimalQuantity_Abstra
     }
 
     public DecimalQuantity_DualStorageBCD(Number number) {
+        // NOTE: Number type expansion happens both here
+        // and in NumberFormat.java
         if (number instanceof Long) {
             setToLong(number.longValue());
         } else if (number instanceof Integer) {

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/NumberFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/NumberFormat.java
index 63d2a109c4092ef679e9b427aab3975feefb0b1e..36779197b0f81c5b9a9b501170e39d11d3174305 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/text/NumberFormat.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/text/NumberFormat.java
@@ -269,6 +269,8 @@ public abstract class NumberFormat extends UFormat {
     public StringBuffer format(Object number,
                                StringBuffer toAppendTo,
                                FieldPosition pos) {
+        // NOTE: Number type expansion happens both here
+        // and in DecimalQuantity_DualStorageBCD.java
         if (number instanceof Long) {
             return format(((Long)number).longValue(), toAppendTo, pos);
         } else if (number instanceof BigInteger) {

diff --git a/icu4j/main/classes/core/src/com/ibm/icu/text/PluralFormat.java b/icu4j/main/classes/core/src/com/ibm/icu/text/PluralFormat.java
index 1918283dc5c1702c4772d5b9501e3c042ae77686..1d9afb97d708a05b940a9f997265ae2f35852a58 100644 (file)
--- a/icu4j/main/classes/core/src/com/ibm/icu/text/PluralFormat.java
+++ b/icu4j/main/classes/core/src/com/ibm/icu/text/PluralFormat.java
@@ -17,6 +17,8 @@ import java.util.Locale;
 import java.util.Map;
 
 import com.ibm.icu.impl.Utility;
+import com.ibm.icu.number.FormattedNumber;
+import com.ibm.icu.number.LocalizedNumberFormatter;
 import com.ibm.icu.text.PluralRules.FixedDecimal;
 import com.ibm.icu.text.PluralRules.IFixedDecimal;
 import com.ibm.icu.text.PluralRules.PluralType;
@@ -613,17 +615,28 @@ public class PluralFormat extends UFormat {
         // Select it based on the formatted number-offset.
         double numberMinusOffset = number - offset;
         String numberString;
-        if (offset == 0) {
-            numberString = numberFormat.format(numberObject);  // could be BigDecimal etc.
-        } else {
-            numberString = numberFormat.format(numberMinusOffset);
-        }
         IFixedDecimal dec;
         if(numberFormat instanceof DecimalFormat) {
-            dec = ((DecimalFormat) numberFormat).getFixedDecimal(numberMinusOffset);
+            // Call NumberFormatter to get both the DecimalQuantity and the string.
+            LocalizedNumberFormatter f = ((DecimalFormat) numberFormat).toNumberFormatter();
+            FormattedNumber result;
+            if (offset == 0) {
+                // could be BigDecimal etc.
+                result  = f.format(numberObject);
+            } else {
+                result  = f.format(numberMinusOffset);
+            }
+            numberString = result.toString();
+            dec = result.getFixedDecimal();
         } else {
+            if (offset == 0) {
+                numberString = numberFormat.format(numberObject);
+            } else {
+                numberString = numberFormat.format(numberMinusOffset);
+            }
             dec = new FixedDecimal(numberMinusOffset);
         }
+
         int partIndex = findSubMessage(msgPattern, 0, pluralRulesWrapper, dec, number);
         // Replace syntactic # signs in the top level of this sub-message
         // (not in nested arguments) with the formatted number-offset.