[GitHub] [zookeeper] maoling commented on a change in pull request #1217: ZOOKEEPER-3691: Use JDK String Join Method in ZK StringUtils

[GitBox <gi...@apache.org>]

maoling commented on a change in pull request #1217: ZOOKEEPER-3691: Use JDK String Join Method in ZK StringUtils
URL: https://github.com/apache/zookeeper/pull/1217#discussion_r365506039
 
 

 ##########
 File path: zookeeper-server/src/main/java/org/apache/zookeeper/common/StringUtils.java
 ##########
 @@ -44,22 +45,24 @@ private StringUtils() {/** non instantiable and non inheritable **/}
     }
 
     /**
-     * This method takes a List<String> and a delimiter and joins the strings
-     * into a single string, where the original strings are separated using
-     * the given delimiter.
+     * This method takes a List<String> and a delimiter and joins the
+     * strings into a single string, where the original strings are separated
+     * using the given delimiter. This method is a null-safe version of
+     * {@link String#join(CharSequence, Iterable)}
      *
+     * <p>
+     * Note that if an individual element is null, then "null" is added.
+     * </p>
+     * @param list a {@code List} that will have its elements joined together
+     * @param delim a sequence of characters that is used to separate each of the
+     *          elements in the resulting String
+     * @return a new String that is composed from the elements argument or
+     *         {@code null} if list is {@code null}
+     * @throws NullPointerException if delim is {@code null}
      */
     public static String joinStrings(List<String> list, String delim) {
-        if (list == null) {
-            return null;
-        }
-
-        StringBuilder builder = new StringBuilder(list.get(0));
-        for (String s : list.subList(1, list.size())) {
-            builder.append(delim).append(s);
-        }
-
-        return builder.toString();
+        Objects.requireNonNull(delim);
+        return list == null ? null : String.join(delim, list);
     }
 
 }
 
 Review comment:
   - Good catch.
   - Could you please add an unit case to assert this change?

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


With regards,
Apache Git Services