Skip to content

Commit 3494eb4

Browse files
magicwerkmagicwerk
committed
implement review comments
1 parent e1b6f15 commit 3494eb4

File tree

3 files changed

+126
-185
lines changed

3 files changed

+126
-185
lines changed

‎log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterFormatterTest.java‎

Lines changed: 28 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,15 @@
2323
import java.util.Collections;
2424
import java.util.List;
2525
import java.util.stream.Collectors;
26+
import java.util.stream.Stream;
2627
import org.apache.logging.log4j.Level;
2728
import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis;
2829
import org.apache.logging.log4j.status.StatusData;
2930
import org.apache.logging.log4j.test.ListStatusListener;
3031
import org.apache.logging.log4j.test.junit.UsingStatusListener;
3132
import org.junit.jupiter.api.Test;
3233
import org.junit.jupiter.params.ParameterizedTest;
34+
import org.junit.jupiter.params.provider.Arguments;
3335
import org.junit.jupiter.params.provider.CsvSource;
3436
import org.junit.jupiter.params.provider.MethodSource;
3537

@@ -146,6 +148,18 @@ static Object[][] messageFormattingTestCases() {
146148
};
147149
}
148150

151+
@Test
152+
void testIdentityToString() {
153+
final List<Object> list = new ArrayList<>();
154+
list.add(1);
155+
// noinspection CollectionAddedToSelf
156+
list.add(list);
157+
list.add(2);
158+
final String actual = ParameterFormatter.identityToString(list);
159+
final String expected = list.getClass().getName() + "@" + Integer.toHexString(System.identityHashCode(list));
160+
assertThat(actual).isEqualTo(expected);
161+
}
162+
149163
@Test
150164
void testDeepToString() {
151165
final List<Object> list = new ArrayList<>();
@@ -172,63 +186,22 @@ void testDeepToStringUsingNonRecursiveButConsequentObjects() {
172186
assertThat(actual).isEqualTo(expected);
173187
}
174188

175-
@Test
176-
void testIdentityToString() {
177-
final List<Object> list = new ArrayList<>();
178-
list.add(1);
179-
// noinspection CollectionAddedToSelf
180-
list.add(list);
181-
list.add(2);
182-
final String actual = ParameterFormatter.identityToString(list);
183-
final String expected = list.getClass().getName() + "@" + Integer.toHexString(System.identityHashCode(list));
184-
assertThat(actual).isEqualTo(expected);
185-
}
186-
187-
@Test
188-
void testDeepToStringArrayInt() {
189-
final int[] array = new int[] {0, 1, 2, 3, 4};
190-
final String actual = ParameterFormatter.deepToString(array);
191-
final String expected = "[0, 1, 2, 3, 4]";
192-
assertThat(actual).isEqualTo(expected);
193-
}
194-
195-
@Test
196-
void testDeepToStringArrayLong() {
197-
final long[] array = new long[] {0, 1, 2, 3, 4};
198-
final String actual = ParameterFormatter.deepToString(array);
199-
final String expected = "[0, 1, 2, 3, 4]";
200-
assertThat(actual).isEqualTo(expected);
201-
}
202-
203-
@Test
204-
void testDeepToStringArrayFloat() {
205-
final float[] array = new float[] {0, 1, 2, 3, 4};
206-
final String actual = ParameterFormatter.deepToString(array);
207-
final String expected = "[0.0, 1.0, 2.0, 3.0, 4.0]";
208-
assertThat(actual).isEqualTo(expected);
209-
}
210-
211-
@Test
212-
void testDeepToStringArrayDouble() {
213-
final double[] array = new double[] {0, 1, 2, 3, 4};
214-
final String actual = ParameterFormatter.deepToString(array);
215-
final String expected = "[0.0, 1.0, 2.0, 3.0, 4.0]";
216-
assertThat(actual).isEqualTo(expected);
217-
}
218-
219-
@Test
220-
void testDeepToStringArrayBoolean() {
221-
final boolean[] array = new boolean[] {false, true};
222-
final String actual = ParameterFormatter.deepToString(array);
223-
final String expected = "[false, true]";
189+
@ParameterizedTest
190+
@MethodSource("deepToStringArgumentsPrimitiveArrays")
191+
void testDeepToStringPrimitiveArrays(Object obj, String expected) {
192+
final String actual = ParameterFormatter.deepToString(obj);
224193
assertThat(actual).isEqualTo(expected);
225194
}
226195

227-
@Test
228-
void testDeepToStringArrayChar() {
229-
final char[] array = new char[] {'a', 'b', 'c'};
230-
final String actual = ParameterFormatter.deepToString(array);
231-
final String expected = "[a, b, c]";
232-
assertThat(actual).isEqualTo(expected);
196+
static Stream<Arguments> deepToStringArgumentsPrimitiveArrays() {
197+
return Stream.of(
198+
Arguments.of(new byte[] {0, 1, 2, 3, 4}, "[0, 1, 2, 3, 4]"),
199+
Arguments.of(new short[] {0, 1, 2, 3, 4}, "[0, 1, 2, 3, 4]"),
200+
Arguments.of(new int[] {0, 1, 2, 3, 4}, "[0, 1, 2, 3, 4]"),
201+
Arguments.of(new long[] {0, 1, 2, 3, 4}, "[0, 1, 2, 3, 4]"),
202+
Arguments.of(new float[] {0, 1, 2, 3, 4}, "[0.0, 1.0, 2.0, 3.0, 4.0]"),
203+
Arguments.of(new double[] {0, 1, 2, 3, 4}, "[0.0, 1.0, 2.0, 3.0, 4.0]"),
204+
Arguments.of(new char[] {'a', 'b', 'c'}, "[a, b, c]"),
205+
Arguments.of(new boolean[] {false, true}, "[false, true]"));
233206
}
234207
}

‎log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterFormatter.java‎

Lines changed: 48 additions & 128 deletions
Original file line numberDiff line numberDiff line change
@@ -645,210 +645,130 @@ static String identityToString(final Object obj) {
645645
}
646646

647647
/**
648-
* Adds a string representation of the contents of the specified array to the buffer.
649-
*
650-
* @param a the array to convert (not null)
651-
* @param str the {@code StringBuilder} that {@code a} will be appended to
652-
*
653648
* @see Arrays#toString(byte[])
654649
*/
655650
private static void appendArray(final byte[] a, final StringBuilder str) {
656-
int iMax = a.length - 1;
657-
if (iMax == -1) {
651+
int len = a.length;
652+
if (len == 0) {
658653
str.append("[]");
659654
return;
660655
}
661-
662-
str.append('[');
663-
for (int i = 0; ; i++) {
664-
str.append(a[i]);
665-
if (i == iMax) {
666-
str.append(']');
667-
return;
668-
}
669-
str.append(", ");
656+
str.append('[').append(a[0]);
657+
for (int i = 1; i < len; i++) {
658+
str.append(", ").append(a[i]);
670659
}
660+
str.append(']');
671661
}
672662

673663
/**
674-
* Adds a string representation of the contents of the specified array to the buffer.
675-
*
676-
* @param a the array to convert (not null)
677-
* @param str the {@code StringBuilder} that {@code a} will be appended to
678-
*
679664
* @see Arrays#toString(short[])
680665
*/
681666
private static void appendArray(final short[] a, final StringBuilder str) {
682-
int iMax = a.length - 1;
683-
if (iMax == -1) {
667+
int len = a.length;
668+
if (len == 0) {
684669
str.append("[]");
685670
return;
686671
}
687-
688-
str.append('[');
689-
for (int i = 0; ; i++) {
690-
str.append(a[i]);
691-
if (i == iMax) {
692-
str.append(']');
693-
return;
694-
}
695-
str.append(", ");
672+
str.append('[').append(a[0]);
673+
for (int i = 1; i < len; i++) {
674+
str.append(", ").append(a[i]);
696675
}
676+
str.append(']');
697677
}
698678

699679
/**
700-
* Adds a string representation of the contents of the specified array to the buffer.
701-
*
702-
* @param a the array to convert (not null)
703-
* @param str the {@code StringBuilder} that {@code a} will be appended to
704-
*
705680
* @see Arrays#toString(int[])
706681
*/
707682
private static void appendArray(final int[] a, final StringBuilder str) {
708-
int iMax = a.length - 1;
709-
if (iMax == -1) {
683+
int len = a.length;
684+
if (len == 0) {
710685
str.append("[]");
711686
return;
712687
}
713-
714-
str.append('[');
715-
for (int i = 0; ; i++) {
716-
str.append(a[i]);
717-
if (i == iMax) {
718-
str.append(']');
719-
return;
720-
}
721-
str.append(", ");
688+
str.append('[').append(a[0]);
689+
for (int i = 1; i < len; i++) {
690+
str.append(", ").append(a[i]);
722691
}
692+
str.append(']');
723693
}
724694

725695
/**
726-
* Adds a string representation of the contents of the specified array to the buffer.
727-
*
728-
* @param a the array to convert (not null)
729-
* @param str the {@code StringBuilder} that {@code a} will be appended to
730-
*
731696
* @see Arrays#toString(long[])
732697
*/
733698
private static void appendArray(final long[] a, final StringBuilder str) {
734-
int iMax = a.length - 1;
735-
if (iMax == -1) {
699+
int len = a.length;
700+
if (len == 0) {
736701
str.append("[]");
737702
return;
738703
}
739-
740-
str.append('[');
741-
for (int i = 0; ; i++) {
742-
str.append(a[i]);
743-
if (i == iMax) {
744-
str.append(']');
745-
return;
746-
}
747-
str.append(", ");
704+
str.append('[').append(a[0]);
705+
for (int i = 1; i < len; i++) {
706+
str.append(", ").append(a[i]);
748707
}
708+
str.append(']');
749709
}
750710

751711
/**
752-
* Adds a string representation of the contents of the specified array to the buffer.
753-
*
754-
* @param a the array to convert (not null)
755-
* @param str the {@code StringBuilder} that {@code a} will be appended to
756-
*
757712
* @see Arrays#toString(float[])
758713
*/
759714
private static void appendArray(final float[] a, final StringBuilder str) {
760-
int iMax = a.length - 1;
761-
if (iMax == -1) {
715+
int len = a.length;
716+
if (len == 0) {
762717
str.append("[]");
763718
return;
764719
}
765-
766-
str.append('[');
767-
for (int i = 0; ; i++) {
768-
str.append(a[i]);
769-
if (i == iMax) {
770-
str.append(']');
771-
return;
772-
}
773-
str.append(", ");
720+
str.append('[').append(a[0]);
721+
for (int i = 1; i < len; i++) {
722+
str.append(", ").append(a[i]);
774723
}
724+
str.append(']');
775725
}
776726

777727
/**
778-
* Adds a string representation of the contents of the specified array to the buffer.
779-
*
780-
* @param a the array to convert (not null)
781-
* @param str the {@code StringBuilder} that {@code a} will be appended to
782-
*
783728
* @see Arrays#toString(double[])
784729
*/
785730
private static void appendArray(final double[] a, final StringBuilder str) {
786-
int iMax = a.length - 1;
787-
if (iMax == -1) {
731+
int len = a.length;
732+
if (len == 0) {
788733
str.append("[]");
789734
return;
790735
}
791-
792-
str.append('[');
793-
for (int i = 0; ; i++) {
794-
str.append(a[i]);
795-
if (i == iMax) {
796-
str.append(']');
797-
return;
798-
}
799-
str.append(", ");
736+
str.append('[').append(a[0]);
737+
for (int i = 1; i < len; i++) {
738+
str.append(", ").append(a[i]);
800739
}
740+
str.append(']');
801741
}
802742

803743
/**
804-
* Adds a string representation of the contents of the specified array to the buffer.
805-
*
806-
* @param a the array to convert (not null)
807-
* @param str the {@code StringBuilder} that {@code a} will be appended to
808-
*
809744
* @see Arrays#toString(boolean[])
810745
*/
811746
private static void appendArray(final boolean[] a, final StringBuilder str) {
812-
int iMax = a.length - 1;
813-
if (iMax == -1) {
747+
int len = a.length;
748+
if (len == 0) {
814749
str.append("[]");
815750
return;
816751
}
817-
818-
str.append('[');
819-
for (int i = 0; ; i++) {
820-
str.append(a[i]);
821-
if (i == iMax) {
822-
str.append(']');
823-
return;
824-
}
825-
str.append(", ");
752+
str.append('[').append(a[0]);
753+
for (int i = 1; i < len; i++) {
754+
str.append(", ").append(a[i]);
826755
}
756+
str.append(']');
827757
}
828758

829759
/**
830-
* Adds a string representation of the contents of the specified array to the buffer.
831-
*
832-
* @param a the array to convert (not null)
833-
* @param str the {@code StringBuilder} that {@code a} will be appended to
834-
*
835760
* @see Arrays#toString(char[])
836761
*/
837762
private static void appendArray(char[] a, final StringBuilder str) {
838-
int iMax = a.length - 1;
839-
if (iMax == -1) {
763+
int len = a.length;
764+
if (len == 0) {
840765
str.append("[]");
841766
return;
842767
}
843-
844-
str.append('[');
845-
for (int i = 0; ; i++) {
846-
str.append(a[i]);
847-
if (i == iMax) {
848-
str.append(']');
849-
return;
850-
}
851-
str.append(", ");
768+
str.append('[').append(a[0]);
769+
for (int i = 1; i < len; i++) {
770+
str.append(", ").append(a[i]);
852771
}
772+
str.append(']');
853773
}
854774
}

0 commit comments

Comments
 (0)