ICU-7912 commit from review comments (thx)

X-SVN-Rev: 34416

icu4c/source/i18n/fmtable.cpp

@@ -893,8 +893,11 @@ FormattableStreamer::streamOut(ostream& stream, const Formattable& obj)
 
 #endif
 
-/* ---- UFormattable stuff ---- */
+U_NAMESPACE_END
 
+/* ---- UFormattable implementation ---- */
+
+U_NAMESPACE_USE
 
 U_DRAFT UFormattable* U_EXPORT2
 ufmt_open(UErrorCode *status) {
@@ -917,49 +920,24 @@ ufmt_close(UFormattable *fmt) {
 }
 
 U_INTERNAL UFormattableType U_EXPORT2
-ufmt_getType(UFormattable *fmt, UErrorCode *status) {
+ufmt_getType(const UFormattable *fmt, UErrorCode *status) {
   if(U_FAILURE(*status)) {
     return (UFormattableType)-1;
   }
-  Formattable *obj = Formattable::fromUFormattable(fmt);
+  const Formattable *obj = Formattable::fromUFormattable(fmt);
-  switch( obj->getType() ) {
+  return (UFormattableType)obj->getType();
-  case Formattable::kDate:
-    return UFMT_DATE;
-    break;
-  case Formattable::kDouble:
-    return UFMT_DOUBLE;
-    break;
-  case Formattable::kLong:
-    return UFMT_LONG;
-    break;
-  case Formattable::kString:
-    return UFMT_STRING;
-    break;
-  case Formattable::kArray:
-    return UFMT_ARRAY;
-    break;
-  case Formattable::kInt64:
-    return UFMT_INT64;
-    break;
-  case Formattable::kObject:
-    return UFMT_OBJECT;
-    break;
-  default:
-    *status = U_ILLEGAL_ARGUMENT_ERROR;
-    return (UFormattableType)-1; // invalid
-  }
 }
 
 
 U_INTERNAL UBool U_EXPORT2
-ufmt_isNumeric(UFormattable *fmt) {
+ufmt_isNumeric(const UFormattable *fmt) {
-  Formattable *obj = Formattable::fromUFormattable(fmt);
+  const Formattable *obj = Formattable::fromUFormattable(fmt);
   return obj->isNumeric();
 }
 
 U_DRAFT UDate U_EXPORT2
-ufmt_getDate(UFormattable *fmt, UErrorCode *status) {
+ufmt_getDate(const UFormattable *fmt, UErrorCode *status) {
-  Formattable *obj = Formattable::fromUFormattable(fmt);
+  const Formattable *obj = Formattable::fromUFormattable(fmt);
 
   return obj->getDate(*status);
 }
@@ -980,8 +958,8 @@ ufmt_getLong(UFormattable *fmt, UErrorCode *status) {
 
 
 U_DRAFT const void *U_EXPORT2
-ufmt_getObject(UFormattable *fmt, UErrorCode *status) {
+ufmt_getObject(const UFormattable *fmt, UErrorCode *status) {
-  Formattable *obj = Formattable::fromUFormattable(fmt);
+  const Formattable *obj = Formattable::fromUFormattable(fmt);
 
   const void *ret = obj->getObject();
   if( ret==NULL &&
@@ -1013,8 +991,8 @@ ufmt_getUChars(UFormattable *fmt, int32_t *len, UErrorCode *status) {
 }
 
 U_DRAFT int32_t U_EXPORT2
-ufmt_getArrayLength(UFormattable* fmt, UErrorCode *status) {
+ufmt_getArrayLength(const UFormattable* fmt, UErrorCode *status) {
-  Formattable *obj = Formattable::fromUFormattable(fmt);
+  const Formattable *obj = Formattable::fromUFormattable(fmt);
 
   int32_t count;
   (void)obj->getArray(count, *status);
@@ -1057,35 +1035,12 @@ ufmt_getDecNumChars(UFormattable *fmt, int32_t *len, UErrorCode *status) {
   }
 }
 
-// make-a-copy version
-// U_DRAFT int32_t U_EXPORT2
-// ufmt_getDecNumChars(UFormattable *fmt, char *buf, int32_t len, UErrorCode *status) {
-//   if(U_FAILURE(*status)) {
-//     return 0;
-//   }
-//   Formattable *obj = Formattable::fromUFormattable(fmt);
-//   CharString *charString = obj->internalGetCharString(*status);
-//   if(U_FAILURE(*status)) {
-//     return 0;
-//   }
-//   if(charString == NULL) {
-//     *status = U_MEMORY_ALLOCATION_ERROR;
-//     return 0;
-//   } else {
-//     return charString->extract(buf, len, *status);
-//   }
-// }
-
-
 U_DRAFT int64_t U_EXPORT2
 ufmt_getInt64(UFormattable *fmt, UErrorCode *status) {
   Formattable *obj = Formattable::fromUFormattable(fmt);
   return obj->getInt64(*status);
 }
 
-
-U_NAMESPACE_END
-
 #endif /* #if !UCONFIG_NO_FORMATTING */
 
 //eof

icu4c/source/i18n/unicode/fmtable.h

@@ -618,8 +618,8 @@ public:
 
     /**
      * Convert this object pointer to a UFormattable.
-     * @return this object as a UFormattable pointer.   This is an alias to the original UFormattable,
+     * @return this object as a UFormattable pointer.   This is an alias to this object,
-     * and so is only valid while the original argument remains in scope.
+     * and so is only valid while this object remains in scope.
      * @draft ICU 52
      */
     inline UFormattable *toUFormattable();
@@ -729,7 +729,7 @@ inline int32_t Formattable::getLong(UErrorCode* status) const {
 #endif  /* U_HIDE_DEPRECATED_API */
 
 inline UFormattable* Formattable::toUFormattable() {
-  return (UFormattable*)(this);
+  return reinterpret_cast<UFormattable*>(this);
 }
 
 inline Formattable* Formattable::fromUFormattable(UFormattable *fmt) {
@@ -740,7 +740,6 @@ inline const Formattable* Formattable::fromUFormattable(const UFormattable *fmt)
   return reinterpret_cast<const Formattable *>(fmt);
 }
 
-
 U_NAMESPACE_END
 
 #endif /* #if !UCONFIG_NO_FORMATTING */

icu4c/source/i18n/unicode/uformattable.h

@@ -25,8 +25,8 @@
  * See {@link unum_parseToUFormattable} for example code.
  */
 
-#ifndef FORMATTABLE_H
+#ifndef UFORMATTABLE_H
-#define FORMATTABLE_H
+#define UFORMATTABLE_H
 
 #include "unicode/utypes.h"
 
@@ -45,10 +45,10 @@ typedef enum UFormattableType {
   UFMT_DATE = 0, /**< ufmt_getDate() will return without conversion. @see ufmt_getDate*/
   UFMT_DOUBLE,   /**< ufmt_getDouble() will return without conversion.  @see ufmt_getDouble*/
   UFMT_LONG,     /**< ufmt_getLong() will return without conversion. @see ufmt_getLong */
-  UFMT_INT64,    /**< ufmt_getInt64() will return without conversion. @see ufmt_getInt64 */
-  UFMT_OBJECT,   /**< ufmt_getObject() will return without conversion.  @see ufmt_getObject*/
   UFMT_STRING,   /**< ufmt_getUChars() will return without conversion.  @see ufmt_getUChars*/
   UFMT_ARRAY,    /**< ufmt_countArray() and ufmt_getArray() will return the value.  @see ufmt_getArrayItemByIndex */
+  UFMT_INT64,    /**< ufmt_getInt64() will return without conversion. @see ufmt_getInt64 */
+  UFMT_OBJECT,   /**< ufmt_getObject() will return without conversion.  @see ufmt_getObject*/
   UFMT_COUNT     /**< Count of defined UFormattableType values */
 } UFormattableType;
 
@@ -113,7 +113,7 @@ U_NAMESPACE_END
  * @draft ICU 52
  */
 U_DRAFT UFormattableType U_EXPORT2
-ufmt_getType(UFormattable* fmt, UErrorCode *status);
+ufmt_getType(const UFormattable* fmt, UErrorCode *status);
 
 /**
  * Return whether the object is numeric.
@@ -124,7 +124,7 @@ ufmt_getType(UFormattable* fmt, UErrorCode *status);
  * @draft ICU 52
  */
 U_DRAFT UBool U_EXPORT2
-ufmt_isNumeric(UFormattable* fmt);
+ufmt_isNumeric(const UFormattable* fmt);
 
 /**
  * Gets the UDate value of this object.  If the type is not of type UFMT_DATE,
@@ -137,7 +137,7 @@ ufmt_isNumeric(UFormattable* fmt);
  * @see icu::Formattable::getDate(UErrorCode&) const
  */
 U_DRAFT UDate U_EXPORT2
-ufmt_getDate(UFormattable* fmt, UErrorCode *status);
+ufmt_getDate(const UFormattable* fmt, UErrorCode *status);
 
 /**
  * Gets the double value of this object. If the type is not a UFMT_DOUBLE, or
@@ -210,7 +210,7 @@ ufmt_getInt64(UFormattable* fmt, UErrorCode *status);
  * @see icu::Formattable::getObject() const
  */
 U_DRAFT const void *U_EXPORT2
-ufmt_getObject(UFormattable* fmt, UErrorCode *status);
+ufmt_getObject(const UFormattable* fmt, UErrorCode *status);
 
 /**
  * Gets the string value of this object as a UChar string. If the type is not a
@@ -236,7 +236,7 @@ ufmt_getUChars(UFormattable* fmt, int32_t *len, UErrorCode *status);
  * @see ufmt_getArrayItemByIndex
  */
 U_DRAFT int32_t U_EXPORT2
-ufmt_getArrayLength(UFormattable* fmt, UErrorCode *status);
+ufmt_getArrayLength(const UFormattable* fmt, UErrorCode *status);
 
 /**
  * Get the specified value from the array of UFormattables. Invalid if the object is not an array type UFMT_ARRAY

icu4c/source/i18n/unicode/unum.h

@@ -559,7 +559,7 @@ unum_formatDoubleCurrency(const UNumberFormat* fmt,
                           UChar* currency,
                           UChar* result,
                           int32_t resultLength,
-                          UFieldPosition* pos, /* ignored if 0 */
+                          UFieldPosition* pos,
                           UErrorCode* status);
 
 /**
@@ -578,7 +578,7 @@ unum_formatDoubleCurrency(const UNumberFormat* fmt,
  * parameter may be NULL, in which case it is ignored.
  * @param status a pointer to an input-output UErrorCode
  * @return the total buffer size needed; if greater than resultLength,
- * the output was truncated.
+ * the output was truncated. Will return 0 on error.
  * @see unum_parseToUFormattable
  * @draft ICU 52
  */
@@ -587,7 +587,7 @@ unum_formatUFormattable(const UNumberFormat* fmt,
                         const UFormattable *number,
                         UChar *result,
                         int32_t resultLength,
-                        UFieldPosition *pos, /* ignored if 0 */
+                        UFieldPosition *pos,
                         UErrorCode *status);
 
 /**

icu4c/source/i18n/unum.cpp

@@ -790,13 +790,20 @@ unum_parseToUFormattable(const UNumberFormat* fmt,
                          int32_t textLength,
                          int32_t* parsePos, /* 0 = start */
                          UErrorCode* status) {
-  if(result == NULL) { // allocate if not allocated.
+  UFormattable *newFormattable = NULL;
-    result = ufmt_open(status); // does an error check
+  if (U_FAILURE(*status)) return result;
+  if (fmt == NULL) {
+    *status = U_ILLEGAL_ARGUMENT_ERROR;
+    return result;
+  }
+  if (result == NULL) { // allocate if not allocated.
+    newFormattable = result = ufmt_open(status);
   }
-  if(U_FAILURE(*status)) return result;
-
   parseRes(*(Formattable::fromUFormattable(result)), fmt, text, textLength, parsePos, status);
-
+  if (U_FAILURE(*status) && newFormattable != NULL) {
+    ufmt_close(newFormattable);
+    result = NULL; // deallocate if there was a parse error
+  }
   return result;
 }
 
@@ -807,16 +814,15 @@ unum_formatUFormattable(const UNumberFormat* fmt,
                         int32_t resultLength,
                         UFieldPosition *pos, /* ignored if 0 */
                         UErrorCode *status) {
-  // cribbed from unum_formatInt64
+    if (U_FAILURE(*status)) {
-    if(U_FAILURE(*status))
+      return 0;
-        return -1;
-
-    UnicodeString res;
-    if(!(result==NULL && resultLength==0)) {
-        // NULL destination for pure preflighting: empty dummy string
-        // otherwise, alias the destination buffer
-        res.setTo(result, 0, resultLength);
     }
+    if (fmt == NULL || number==NULL ||
+        (result==NULL ? resultLength!=0 : resultLength<0)) {
+      *status = U_ILLEGAL_ARGUMENT_ERROR;
+      return 0;
+    }
+    UnicodeString res(result, 0, resultLength);
 
     FieldPosition fp;
 

icu4c/source/test/cintltst/cnumtst.c

@@ -2188,7 +2188,7 @@ static void TestUFormattable(void) {
   {
     UErrorCode status = U_ZERO_ERROR;
     UNumberFormat *unum = unum_open(UNUM_DEFAULT, NULL, -1, "en_US_POSIX", NULL, &status);
-    if(assertSuccessCheck("calling ufmt_open()", &status, TRUE)) {
+    if(assertSuccessCheck("calling unum_open()", &status, TRUE)) {
       //! [unum_parseToUFormattable]
       const UChar str[] = { 0x0031, 0x0032, 0x0033, 0x0000 }; /* 123 */
       int32_t result = 0;
@@ -2213,7 +2213,7 @@ static void TestUFormattable(void) {
 
     ufmt = ufmt_open(&status);
     unum = unum_open(UNUM_DEFAULT, NULL, -1, "en_US_POSIX", NULL, &status);
-    if(assertSuccessCheck("calling ufmt_open()", &status, TRUE)) {
+    if(assertSuccessCheck("calling ufmt_open() || unum_open()", &status, TRUE)) {
 
       pattern = "31337";
       log_verbose("-- pattern: %s\n", pattern);
@@ -2261,11 +2261,11 @@ static void TestUFormattable(void) {
     u_uastrcpy(buffer, pattern);
 
     unum = unum_open(UNUM_DEFAULT, NULL, -1, "en_US_POSIX", NULL, &status);
-    if(assertSuccessCheck("calling ufmt_open()", &status, TRUE)) {
+    if(assertSuccessCheck("calling unum_open()", &status, TRUE)) {
 
-      ufmt = unum_parseToUFormattable(unum, NULL, /* will be unum_open()'ed for us */
+      ufmt = unum_parseToUFormattable(unum, NULL, /* will be ufmt_open()'ed for us */
                                    buffer, -1, NULL, &status);
-      if(assertSuccess("unum_parseToUFormattable(weight of the moon", &status)) {
+      if(assertSuccess("unum_parseToUFormattable(weight of the moon)", &status)) {
         log_verbose("new formattable allocated at %p\n", (void*)ufmt);
         assertTrue("ufmt_isNumeric() TRUE", ufmt_isNumeric(ufmt));
         unum_formatUFormattable(unum, ufmt, out2k, 2048, NULL, &status);

icu4c/source/test/intltest/intltest.cpp

@@ -218,7 +218,7 @@ UnicodeString toString(int32_t n) {
 
 
 UnicodeString toString(UBool b) {
-  return b ? UnicodeString("TRUE"):UnicodeString("false");
+  return b ? UnicodeString("TRUE"):UnicodeString("FALSE");
 }
 
 // stephen - cleaned up 05/05/99

icu4c/source/test/intltest/intltest.h

@@ -51,7 +51,7 @@ U_NAMESPACE_USE
 //string-concatenation operator (moved from findword test by rtg)
 UnicodeString UCharToUnicodeString(UChar c);
 UnicodeString Int64ToUnicodeString(int64_t num);
-//UnicodeString operator+(const UnicodeString& left, int64_t num);
+//UnicodeString operator+(const UnicodeString& left, int64_t num); // Some compilers don't allow this because of the long type.
 UnicodeString operator+(const UnicodeString& left, long num);
 UnicodeString operator+(const UnicodeString& left, unsigned long num);
 UnicodeString operator+(const UnicodeString& left, double num);

icu4c/source/test/intltest/numfmtst.cpp

@@ -6832,6 +6832,7 @@ UBool NumberFormatTest::testFormattableAsUFormattable(const char *file, int line
       const UChar* uch = ufmt_getUChars(u, &len, &valueStatus);
       if(U_SUCCESS(valueStatus)) {
         UnicodeString str2(uch, len);
+        assertTrue("UChar* NULL-terminated", uch[len]==0);
         exactMatch = (str == str2);
       }
       triedExact = TRUE;