garydgregory commented on code in PR #838:
URL: https://github.com/apache/commons-lang/pull/838#discussion_r843999518
##########
src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java:
##########
@@ -99,6 +107,34 @@ public ReflectionDiffBuilder(final T lhs, final T rhs,
final ToStringStyle style
diffBuilder = new DiffBuilder<>(lhs, rhs, style);
}
+ /**
+ * Returns the field names that should be excluded from the diff
+ * @return Returns the excludeFieldNames.
+ * @since 3.13.0
+ */
+ public String[] getExcludeFieldNames() {
+ return this.excludeFieldNames.clone();
+ }
+
+
+ /**
+ * Sets the field names to exclude.
+ *
+ * @param excludeFieldNamesParam
+ * The field names to exclude from the diff or {@code null}.
+ * @return {@code this}
+ * @since 3.13.0
+ */
+ public ReflectionDiffBuilder setExcludeFieldNames(final String...
excludeFieldNamesParam) {
+ if (excludeFieldNamesParam == null) {
+ this.excludeFieldNames = new String[0];
Review Comment:
Reuse `ArrayUtils.EMPTY_STRING_ARRAY`
##########
src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java:
##########
@@ -73,6 +75,12 @@
private final Object right;
private final DiffBuilder<T> diffBuilder;
+ /**
+ * Which field names to exclude from output. Intended for fields like
{@code "password"} or {@code "lastModificationDate}.
+ * @since 3.13.0
+ */
+ protected String[] excludeFieldNames;
Review Comment:
Make `private`.
##########
src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java:
##########
@@ -99,6 +107,34 @@ public ReflectionDiffBuilder(final T lhs, final T rhs,
final ToStringStyle style
diffBuilder = new DiffBuilder<>(lhs, rhs, style);
}
+ /**
+ * Returns the field names that should be excluded from the diff
+ * @return Returns the excludeFieldNames.
+ * @since 3.13.0
+ */
+ public String[] getExcludeFieldNames() {
+ return this.excludeFieldNames.clone();
+ }
+
+
+ /**
+ * Sets the field names to exclude.
+ *
+ * @param excludeFieldNamesParam
+ * The field names to exclude from the diff or {@code null}.
+ * @return {@code this}
+ * @since 3.13.0
+ */
+ public ReflectionDiffBuilder setExcludeFieldNames(final String...
excludeFieldNamesParam) {
+ if (excludeFieldNamesParam == null) {
+ this.excludeFieldNames = new String[0];
+ } else {
+ //clone and remove nulls
Review Comment:
Space after `//`
##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -48,10 +48,14 @@
private final Object[] objectArrayField = {null};
private static int staticField;
private transient String transientField;
+ @DiffExclude
+ private String annotatedField = "a";
+ private String excludedField = "a";
Review Comment:
You should test a non-transient null field.
##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -92,7 +96,7 @@ public void test_primitive_difference() {
@Test
public void test_array_difference() {
final TypeTestClass firstObject = new TypeTestClass();
- firstObject.charArrayField = new char[] { 'c' };
+ firstObject.charArrayField = new char[]{'c'};
Review Comment:
Leave formatting alone here.
##########
src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java:
##########
@@ -131,7 +167,15 @@ private boolean accept(final Field field) {
if (Modifier.isTransient(field.getModifiers())) {
return false;
}
- return !Modifier.isStatic(field.getModifiers());
+ if (Modifier.isStatic(field.getModifiers())) {
+ return false;
+ }
+ if (this.excludeFieldNames != null
Review Comment:
If you make the default value the empty array and the setter does not allow
excludeFieldNames to be null, then you do not need this check? It might be
better this way though, not sure.
##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -128,4 +132,43 @@ public void test_difference_in_inherited_field() {
final DiffResult list = firstObject.diff(secondObject);
assertEquals(1, list.getNumberOfDiffs());
}
+
+ @Test
+ public void test_no_differences_excluded_field() {
+ final TypeTestClass firstObject = new TypeTestClass();
+ firstObject.excludedField = "b";
+ final TypeTestClass secondObject = new TypeTestClass();
+
+ final DiffResult list = firstObject.diff(secondObject);
+ assertEquals(0, list.getNumberOfDiffs());
+ }
+
+ @Test
+ public void test_no_differences_diff_exclude_annotated_field() {
+ final TypeTestClass firstObject = new TypeTestClass();
+ firstObject.annotatedField = "b";
+ final TypeTestClass secondObject = new TypeTestClass();
+
+ final DiffResult list = firstObject.diff(secondObject);
+ assertEquals(0, list.getNumberOfDiffs());
+ }
+
+ @Test
+ public void testGetExcludeFieldNamesWithNullExcludedFieldNames() {
+ ReflectionDiffBuilder<TypeTestClass> reflectionDiffBuilder = new
ReflectionDiffBuilder<>(new TypeTestClass(), new TypeTestChildClass(),
SHORT_STYLE);
+ reflectionDiffBuilder.setExcludeFieldNames(null);
+ String[] excludeFieldNames =
reflectionDiffBuilder.getExcludeFieldNames();
+ assertNotNull(excludeFieldNames);
+ assertEquals(0, excludeFieldNames.length);
+ }
+
+ @Test
+ public void testGetExcludeFieldNamesWithNullValuesInExcludedFieldNames() {
+ ReflectionDiffBuilder<TypeTestClass> reflectionDiffBuilder = new
ReflectionDiffBuilder<>(new TypeTestClass(), new TypeTestChildClass(),
SHORT_STYLE);
Review Comment:
Use final where you can.
##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -128,4 +132,43 @@ public void test_difference_in_inherited_field() {
final DiffResult list = firstObject.diff(secondObject);
assertEquals(1, list.getNumberOfDiffs());
}
+
+ @Test
+ public void test_no_differences_excluded_field() {
+ final TypeTestClass firstObject = new TypeTestClass();
+ firstObject.excludedField = "b";
+ final TypeTestClass secondObject = new TypeTestClass();
+
+ final DiffResult list = firstObject.diff(secondObject);
+ assertEquals(0, list.getNumberOfDiffs());
+ }
+
+ @Test
+ public void test_no_differences_diff_exclude_annotated_field() {
+ final TypeTestClass firstObject = new TypeTestClass();
+ firstObject.annotatedField = "b";
+ final TypeTestClass secondObject = new TypeTestClass();
+
+ final DiffResult list = firstObject.diff(secondObject);
+ assertEquals(0, list.getNumberOfDiffs());
+ }
+
+ @Test
+ public void testGetExcludeFieldNamesWithNullExcludedFieldNames() {
+ ReflectionDiffBuilder<TypeTestClass> reflectionDiffBuilder = new
ReflectionDiffBuilder<>(new TypeTestClass(), new TypeTestChildClass(),
SHORT_STYLE);
Review Comment:
Use final where you can.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
