From ca65e1c41e21bb9020c524a2ada5ec3726f54c28 Mon Sep 17 00:00:00 2001
From: Markus Scherer <markus.icu@gmail.com>
Date: Wed, 8 Aug 2012 21:03:52 +0000
Subject: [PATCH] ICU-9460 deprecate Collator::safeClone() and
 default-implement it in the base class

X-SVN-Rev: 32127
---
 icu4c/source/i18n/coll.cpp          |  5 +++
 icu4c/source/i18n/tblcoll.cpp       | 58 +++++++++--------------------
 icu4c/source/i18n/unicode/coll.h    | 15 ++++----
 icu4c/source/i18n/unicode/tblcoll.h | 12 +-----
 4 files changed, 33 insertions(+), 57 deletions(-)

diff --git a/icu4c/source/i18n/coll.cpp b/icu4c/source/i18n/coll.cpp
index 9ba9a6dbd6..9e9ca0239d 100644
--- a/icu4c/source/i18n/coll.cpp
+++ b/icu4c/source/i18n/coll.cpp
@@ -425,6 +425,11 @@ Collator::createInstance(const Locale &loc,
 }
 #endif
 
+Collator *
+Collator::safeClone() const {
+    return clone();
+}
+
 // implement deprecated, previously abstract method
 Collator::EComparisonResult Collator::compare(const UnicodeString& source, 
                                     const UnicodeString& target) const
diff --git a/icu4c/source/i18n/tblcoll.cpp b/icu4c/source/i18n/tblcoll.cpp
index 8e23b956d7..96ff6875f2 100644
--- a/icu4c/source/i18n/tblcoll.cpp
+++ b/icu4c/source/i18n/tblcoll.cpp
@@ -206,32 +206,33 @@ UBool RuleBasedCollator::operator==(const Collator& that) const
 // aliasing, not write-through
 RuleBasedCollator& RuleBasedCollator::operator=(const RuleBasedCollator& that)
 {
-    if (this != &that)
-    {
-        if (dataIsOwned)
-        {
-            ucol_close(ucollator);
-        }
+    if (this == &that) { return *this; }
 
-        urulestring.truncate(0); // empty the rule string
-        dataIsOwned = TRUE;
-        isWriteThroughAlias = FALSE;
+    UErrorCode intStatus = U_ZERO_ERROR;
+    int32_t buffersize = U_COL_SAFECLONE_BUFFERSIZE;
+    UCollator *ucol = ucol_safeClone(that.ucollator, NULL, &buffersize, &intStatus);
+    if (U_FAILURE(intStatus)) { return *this; }
 
-        UErrorCode intStatus = U_ZERO_ERROR;
-        int32_t buffersize = U_COL_SAFECLONE_BUFFERSIZE;
-        ucollator = ucol_safeClone(that.ucollator, NULL, &buffersize,
-                                        &intStatus);
-        if (U_SUCCESS(intStatus)) {
-            setRuleStringFromCollator();
-        }
+    if (dataIsOwned) {
+        ucol_close(ucollator);
     }
+    ucollator = ucol;
+    dataIsOwned = TRUE;
+    isWriteThroughAlias = FALSE;
+    setRuleStringFromCollator();
     return *this;
 }
 
 // aliasing, not write-through
 Collator* RuleBasedCollator::clone() const
 {
-    return new RuleBasedCollator(*this);
+    RuleBasedCollator* coll = new RuleBasedCollator(*this);
+    // There is a small chance that the internal ucol_safeClone() call fails.
+    if (coll != NULL && coll->ucollator == NULL) {
+        delete coll;
+        return NULL;
+    }
+    return coll;
 }
 
 
@@ -519,29 +520,6 @@ uint32_t RuleBasedCollator::getVariableTop(UErrorCode &status) const {
   return ucol_getVariableTop(ucollator, &status);
 }
 
-Collator* RuleBasedCollator::safeClone(void) const
-{
-    UErrorCode intStatus = U_ZERO_ERROR;
-    int32_t buffersize = U_COL_SAFECLONE_BUFFERSIZE;
-    UCollator *ucol = ucol_safeClone(ucollator, NULL, &buffersize,
-                                    &intStatus);
-    if (U_FAILURE(intStatus)) {
-        return NULL;
-    }
-
-    RuleBasedCollator *result = new RuleBasedCollator();
-    // Null pointer check
-    if (result != NULL) {
-        result->ucollator = ucol;
-        result->dataIsOwned = TRUE;
-        result->isWriteThroughAlias = FALSE;
-        result->setRuleStringFromCollator();
-    }
-
-    return result;
-}
-
-
 int32_t RuleBasedCollator::getSortKey(const UnicodeString& source,
                                          uint8_t *result, int32_t resultLength)
                                          const
diff --git a/icu4c/source/i18n/unicode/coll.h b/icu4c/source/i18n/unicode/coll.h
index b1d802809c..5a5a4d4085 100644
--- a/icu4c/source/i18n/unicode/coll.h
+++ b/icu4c/source/i18n/unicode/coll.h
@@ -270,8 +270,8 @@ public:
     virtual UBool operator!=(const Collator& other) const;
 
     /**
-     * Makes a shallow copy of the current object.
-     * @return a copy of this object
+     * Makes a copy of this object.
+     * @return a copy of this object, owned by the caller
      * @stable ICU 2.0
      */
     virtual Collator* clone(void) const = 0;
@@ -933,13 +933,14 @@ public:
      */
     virtual UnicodeSet *getTailoredSet(UErrorCode &status) const;
 
-
     /**
-     * Thread safe cloning operation
-     * @return pointer to the new clone, user should remove it.
-     * @stable ICU 2.2
+     * Same as clone().
+     * The base class implementation simply calls clone().
+     * @return a copy of this object, owned by the caller
+     * @see clone()
+     * @deprecated ICU 50 no need to have two methods for cloning
      */
-    virtual Collator* safeClone(void) const = 0;
+    virtual Collator* safeClone(void) const;
 
     /**
      * Get the sort key as an array of bytes from an UnicodeString.
diff --git a/icu4c/source/i18n/unicode/tblcoll.h b/icu4c/source/i18n/unicode/tblcoll.h
index f08ee599ec..9d62fa6136 100644
--- a/icu4c/source/i18n/unicode/tblcoll.h
+++ b/icu4c/source/i18n/unicode/tblcoll.h
@@ -224,9 +224,8 @@ public:
     virtual UBool operator==(const Collator& other) const;
 
     /**
-     * Makes a deep copy of the object.
-     * The caller owns the returned object.
-     * @return the cloned object.
+     * Makes a copy of this object.
+     * @return a copy of this object, owned by the caller
      * @stable ICU 2.0
      */
     virtual Collator* clone(void) const;
@@ -536,13 +535,6 @@ public:
      */
     virtual UnicodeSet *getTailoredSet(UErrorCode &status) const;
 
-    /**
-     * Thread safe cloning operation.
-     * @return pointer to the new clone, user should remove it.
-     * @stable ICU 2.2
-     */
-    virtual Collator* safeClone(void) const;
-
     /**
      * Get the sort key as an array of bytes from an UnicodeString.
      * @param source string to be processed.