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


##########
src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java:
##########
@@ -115,6 +116,11 @@ private static void reflectionAppend(
         final boolean useTransients,
         final String[] excludeFields) {
 
+        if ((lhs instanceof CharSequence || lhs instanceof Number || lhs 
instanceof Temporal) && lhs instanceof Comparable) {

Review Comment:
   Needs a // comment IMO



##########
src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java:
##########
@@ -867,4 +874,21 @@ private void validate() {
         }
     }
 
+    private boolean handleNativeClasses() {
+        Object value = getObject();
+        if (value instanceof Number || value instanceof Boolean || value 
instanceof Character
+            || value instanceof TemporalAccessor ) {

Review Comment:
   Hm, why `TemporalAccessor` here and `Temporal` above?



##########
src/main/java/org/apache/commons/lang3/builder/CompareToBuilder.java:
##########
@@ -115,6 +116,11 @@ private static void reflectionAppend(
         final boolean useTransients,
         final String[] excludeFields) {
 
+        if ((lhs instanceof CharSequence || lhs instanceof Number || lhs 
instanceof Temporal) && lhs instanceof Comparable) {
+            builder.append(lhs, rhs);
+            return;
+        }
+

Review Comment:
   Extra blank lines not needed IMO.



##########
src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java:
##########
@@ -731,7 +732,8 @@ public EqualsBuilder append(final Object lhs, final Object 
rhs) {
             // to be inlined
             appendArray(lhs, rhs);
         } else // The simple case, not an array, just test the element
-        if (testRecursive && !ClassUtils.isPrimitiveOrWrapper(lhsClass)) {
+        if (testRecursive && !ClassUtils.isPrimitiveOrWrapper(lhsClass)

Review Comment:
   Needs a // comment IMO



##########
src/main/java/org/apache/commons/lang3/builder/HashCodeBuilder.java:
##########
@@ -180,6 +181,10 @@ private static void reflectionAppend(final Object object, 
final Class<?> clazz,
         if (isRegistered(object)) {
             return;
         }
+        if (object instanceof CharSequence || object instanceof Number || 
object instanceof Temporal) {

Review Comment:
   Needs a // comment IMO. Why not refactor the test exporession into a common 
(if possible package-private) method with the same code above?



##########
src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java:
##########
@@ -867,4 +874,21 @@ private void validate() {
         }
     }
 
+    private boolean handleNativeClasses() {
+        Object value = getObject();
+        if (value instanceof Number || value instanceof Boolean || value 
instanceof Character
+            || value instanceof TemporalAccessor ) {
+            getStringBuffer().append("value=").append(getObject().toString());
+            return true;
+        }
+        
+        if (value.getClass().isArray() || value instanceof Collection || value 
instanceof Map) {
+            ToStringStyle.unregister(value);
+            getStyle().append(getStringBuffer(), null, value, true);
+            return true;
+        }
+        

Review Comment:
   Extra empty line not needed IMO.  Want I want call attention to something, I 
write a // comment.



##########
src/main/java/org/apache/commons/lang3/builder/EqualsBuilder.java:
##########
@@ -956,7 +958,8 @@ public EqualsBuilder reflectionAppend(final Object lhs, 
final Object rhs) {
         }
 
         try {
-            if (testClass.isArray()) {
+            if (testClass.isArray() || Number.class.isAssignableFrom(testClass)

Review Comment:
   Needs a // comment IMO



##########
src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java:
##########
@@ -867,4 +874,21 @@ private void validate() {
         }
     }
 
+    private boolean handleNativeClasses() {
+        Object value = getObject();
+        if (value instanceof Number || value instanceof Boolean || value 
instanceof Character
+            || value instanceof TemporalAccessor ) {
+            getStringBuffer().append("value=").append(getObject().toString());
+            return true;
+        }
+        

Review Comment:
   Extra blank line not needed IMO. Want I want call attention to something, I 
write a // comment.



##########
src/main/java/org/apache/commons/lang3/builder/ReflectionToStringBuilder.java:
##########
@@ -867,4 +874,21 @@ private void validate() {
         }
     }
 
+    private boolean handleNativeClasses() {
+        Object value = getObject();
+        if (value instanceof Number || value instanceof Boolean || value 
instanceof Character
+            || value instanceof TemporalAccessor ) {
+            getStringBuffer().append("value=").append(getObject().toString());
+            return true;
+        }
+        
+        if (value.getClass().isArray() || value instanceof Collection || value 
instanceof Map) {

Review Comment:
   Can `value` be null here?



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