[ 
https://issues.apache.org/jira/browse/GEODE-3288?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16282072#comment-16282072
 ] 

ASF GitHub Bot commented on GEODE-3288:
---------------------------------------

dgkimura commented on a change in pull request #165: GEODE-3288: Converts 
CacheableString to std::string.
URL: https://github.com/apache/geode-native/pull/165#discussion_r155345137
 
 

 ##########
 File path: cppcache/include/geode/CacheableString.hpp
 ##########
 @@ -105,130 +103,59 @@ class CPPCACHE_EXPORT CacheableString : public 
CacheableKey {
   /** return the hashcode for this key. */
   virtual int32_t hashcode() const override;
 
-  /**
-   * Factory method for creating an instance of CacheableString from
-   * a null terminated C string optionally giving the length.
-   *
-   * This should be used only for ASCII strings.
-   */
-  static std::shared_ptr<CacheableString> create(const char* value,
-                                                 int32_t len = 0) {
-    if (nullptr == value) {
-      return nullptr;
-    }
-
-    auto str = std::make_shared<CacheableString>();
-    str->initString(value, len);
-    return str;
+  inline static std::shared_ptr<CacheableString> create(
+      const std::string& value) {
+    return std::make_shared<CacheableString>(value);
   }
 
-  /**
-   * Factory method for creating an instance of CacheableString from
-   * a C string of given length by taking ownership of the string without
-   * making a copy. The string should have been allocated using
-   * the standard C++ new operator.
-   *
-   * This should be used only for ASCII strings.
-   *
-   * CAUTION: use this only when you really know what you are doing.
-   */
-  static std::shared_ptr<CacheableString> createNoCopy(char* value,
-                                                       int32_t len = 0) {
-    if (nullptr == value) {
-      return nullptr;
-    }
-
-    auto str = std::make_shared<CacheableString>();
-    str->initStringNoCopy(value, len);
-    return str;
+  inline static std::shared_ptr<CacheableString> create(std::string&& value) {
+    return std::make_shared<CacheableString>(std::move(value));
   }
 
-  /**
-   * Factory method for creating an instance of CacheableString from a
-   * wide-character null terminated C string optionally giving the length.
-   *
-   * This should be used for non-ASCII strings.
-   */
-  static std::shared_ptr<CacheableString> create(const wchar_t* value,
-                                                 int32_t len = 0) {
-    if (nullptr == value) {
-      return nullptr;
-    }
+  static std::shared_ptr<CacheableString> create(const std::u16string& value);
 
-    auto str = std::make_shared<CacheableString>();
-    str->initString(value, len);
-    return str;
-  }
+  static std::shared_ptr<CacheableString> create(std::u16string&& value);
 
 Review comment:
   You might be able to use universal reference and std::forward if you want to 
consolidate these into one method.  Maybe something like:
   
   ```cpp
   template<typename T>
   inline static std::shared_ptr<CacheableString> create(T&& value) {
       return std::make_shared<CacheableString>(std::forward(value));
   }
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


> Replace char* with std::string
> ------------------------------
>
>                 Key: GEODE-3288
>                 URL: https://issues.apache.org/jira/browse/GEODE-3288
>             Project: Geode
>          Issue Type: Improvement
>          Components: native client
>            Reporter: Ernest Burghardt
>
> In all public API headers



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to