garydgregory commented on code in PR #1137:
URL: https://github.com/apache/commons-lang/pull/1137#discussion_r1721660740


##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -184,4 +209,18 @@ public void 
testGetExcludeFieldNamesWithNullValuesInExcludedFieldNames() {
         assertEquals("charField", excludeFieldNames[0]);
     }
 
+    @Test
+    public void testEqualsObjectDiff() {
+        final TypeTestEntityClass firstObject = new TypeTestEntityClass();
+        firstObject.id = 1L;
+        firstObject.field1 = "a";
+
+        final TypeTestEntityClass secondObject = new TypeTestEntityClass();
+        secondObject.id = 1L;
+        secondObject.field1 = "b";
+
+        final DiffResult list = firstObject.diff(secondObject);
+        assertEquals(1, list.getNumberOfDiffs());

Review Comment:
   This test should assert _what_ the difference is; IOW, make assertions on 
the other properties of the `DiffResult` and `Diff` instances.



##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -29,6 +29,31 @@ private static final class TypeTestChildClass extends 
TypeTestClass {
         String field = "a";
     }
 
+    @SuppressWarnings("unused")
+    private static class TypeTestEntityClass implements 
Diffable<TypeTestEntityClass> {
+
+        private Long id;
+        private String field1;
+
+        @Override
+        public DiffResult<TypeTestEntityClass> diff(TypeTestEntityClass obj) {
+            return new ReflectionDiffBuilder(this, obj, SHORT_STYLE, 
false).build();
+        }
+
+        @Override
+        public boolean equals(Object obj) {
+            if (this == obj) return true;

Review Comment:
   Always use blocks (`{...}`}



##########
src/main/java/org/apache/commons/lang3/builder/ReflectionDiffBuilder.java:
##########
@@ -103,9 +104,38 @@ public class ReflectionDiffBuilder<T> implements 
Builder<DiffResult<T>> {
      *             if {@code lhs} or {@code rhs} is {@code null}
      */
     public ReflectionDiffBuilder(final T lhs, final T rhs, final ToStringStyle 
style) {
+        this(lhs, rhs, style, true);
+    }
+
+    /**
+     * Constructs a builder for the specified objects with the specified style.
+     *
+     * <p>
+     * If {@code lhs == rhs} or {@code lhs.equals(rhs)} then the builder will
+     * not evaluate any calls to {@code append(...)} and will return an empty
+     * {@link DiffResult} when {@link #build()} is executed.
+     * </p>
+     * @param lhs
+     *            {@code this} object
+     * @param rhs
+     *            the object to diff against
+     * @param style
+     *            the style will use when outputting the objects, {@code null}
+     *            uses the default
+     * @param testTriviallyEqual
+     *            If true, this will test if lhs and rhs are the same or equal.
+     *            All of the append(fieldName, lhs, rhs) methods will abort
+     *            without creating a field {@link Diff} if the trivially equal
+     *            test is enabled and returns true.  The result of this test
+     *            is never changed throughout the life of this {@link 
ReflectionDiffBuilder}
+     * @throws IllegalArgumentException
+     *             if {@code lhs} or {@code rhs} is {@code null}
+     */
+    public ReflectionDiffBuilder(final T lhs, final T rhs, final ToStringStyle 
style, boolean testTriviallyEqual) {

Review Comment:
   Do _not_ add another constructor. This class provides a builder for new 
settings.
   



##########
src/test/java/org/apache/commons/lang3/builder/ReflectionDiffBuilderTest.java:
##########
@@ -29,6 +29,31 @@ private static final class TypeTestChildClass extends 
TypeTestClass {
         String field = "a";
     }
 
+    @SuppressWarnings("unused")
+    private static class TypeTestEntityClass implements 
Diffable<TypeTestEntityClass> {

Review Comment:
   Rename `TypeTestEntityClass` to `EntityTestClass`.
   Add a class Javadoc that states the intent for the test fixture to emulate a 
JPA entity class.



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

Reply via email to