ppkarwasz commented on code in PR #3645:
URL: https://github.com/apache/logging-log4j2/pull/3645#discussion_r2073036752


##########
log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java:
##########
@@ -643,4 +643,212 @@ static String identityToString(final Object obj) {
         }
         return obj.getClass().getName() + '@' + 
Integer.toHexString(System.identityHashCode(obj));
     }
+
+    /**
+     * Adds a string representation of the contents of the specified array to 
the buffer.
+     *
+     * @param a      the array to convert (not null)
+     * @param str    the {@code StringBuilder} that {@code a} will be appended 
to
+     *
+     * @see Arrays#toString(byte[])
+     */
+    private static void appendArray(final byte[] a, final StringBuilder str) {
+        int iMax = a.length - 1;
+        if (iMax == -1) {
+            str.append("[]");
+            return;
+        }
+
+        str.append('[');
+        for (int i = 0; ; i++) {
+            str.append(a[i]);
+            if (i == iMax) {
+                str.append(']');
+                return;
+            }
+            str.append(", ");
+        }
+    }
+
+    /**
+     * Adds a string representation of the contents of the specified array to 
the buffer.
+     *
+     * @param a      the array to convert (not null)
+     * @param str    the {@code StringBuilder} that {@code a} will be appended 
to
+     *
+     * @see Arrays#toString(short[])
+     */
+    private static void appendArray(final short[] a, final StringBuilder str) {
+        int iMax = a.length - 1;
+        if (iMax == -1) {
+            str.append("[]");
+            return;
+        }
+
+        str.append('[');
+        for (int i = 0; ; i++) {
+            str.append(a[i]);
+            if (i == iMax) {
+                str.append(']');
+                return;
+            }
+            str.append(", ");
+        }
+    }
+
+    /**
+     * Adds a string representation of the contents of the specified array to 
the buffer.
+     *
+     * @param a      the array to convert (not null)
+     * @param str    the {@code StringBuilder} that {@code a} will be appended 
to
+     *
+     * @see Arrays#toString(int[])
+     */
+    private static void appendArray(final int[] a, final StringBuilder str) {
+        int iMax = a.length - 1;
+        if (iMax == -1) {
+            str.append("[]");
+            return;
+        }
+
+        str.append('[');
+        for (int i = 0; ; i++) {
+            str.append(a[i]);
+            if (i == iMax) {
+                str.append(']');
+                return;
+            }
+            str.append(", ");
+        }
+    }

Review Comment:
   Looks good, but did you try:
   
   ```java
   int length = a.length;
   str.append('[');
   if (0 < length) {
       str.append(a[0]);
   }
   for (int i = 1; i < length; i++) {
       str.append(", ");
       str.append(a[i]);
   }
   str.append(']');
   ```
   
   According to my benchmarks it is slightly (less than `0.4 %`) faster and 
arguably easier to read.



##########
log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java:
##########
@@ -643,4 +643,212 @@ static String identityToString(final Object obj) {
         }
         return obj.getClass().getName() + '@' + 
Integer.toHexString(System.identityHashCode(obj));
     }
+
+    /**
+     * Adds a string representation of the contents of the specified array to 
the buffer.
+     *
+     * @param a      the array to convert (not null)
+     * @param str    the {@code StringBuilder} that {@code a} will be appended 
to
+     *
+     * @see Arrays#toString(byte[])
+     */

Review Comment:
   Since this is a `private` method the JavaDoc isn't really necessary. The 
`@see Arrays#toString(byte[])` should be enough.



##########
log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java:
##########
@@ -183,4 +183,52 @@ void testIdentityToString() {
         final String expected = list.getClass().getName() + "@" + 
Integer.toHexString(System.identityHashCode(list));
         assertThat(actual).isEqualTo(expected);
     }
+
+    @Test
+    void testDeepToStringArrayInt() {
+        final int[] array = new int[] {0, 1, 2, 3, 4};
+        final String actual = ParameterFormatter.deepToString(array);
+        final String expected = "[0, 1, 2, 3, 4]";
+        assertThat(actual).isEqualTo(expected);
+    }

Review Comment:
   Could you use a single parameterized test instead of multiple test cases?
   
   Note: the parameterized test could also include the existing 
`testDeepToString` and `testDeepToStringUsingNonRecursiveButConsequentObjects` 
test cases.



-- 
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.

To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to