garydgregory commented on code in PR #276:
URL: https://github.com/apache/commons-beanutils/pull/276#discussion_r1730155019


##########
src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java:
##########
@@ -380,29 +380,7 @@ public void copyProperty(final Object bean, String name, 
Object value)
         // Trace logging (if enabled)
         if (LOG.isTraceEnabled()) {
             final StringBuilder sb = new StringBuilder("  copyProperty(");
-            sb.append(bean);
-            sb.append(", ");
-            sb.append(name);
-            sb.append(", ");
-            if (value == null) {
-                sb.append("<NULL>");
-            } else if (value instanceof String) {
-                sb.append((String) value);
-            } else if (value instanceof String[]) {
-                final String[] values = (String[]) value;
-                sb.append('[');
-                for (int i = 0; i < values.length; i++) {
-                    if (i > 0) {
-                        sb.append(',');
-                    }
-                    sb.append(values[i]);
-                }
-                sb.append(']');
-            } else {
-                sb.append(value.toString());
-            }
-            sb.append(')');
-            LOG.trace(sb.toString());
+            LOG.trace(traceLogRecord(bean, name, value, sb).toString());;

Review Comment:
   Too many semicolons;;



##########
src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java:
##########
@@ -911,29 +889,7 @@ public void setProperty(final Object bean, String name, 
final Object value)
         // Trace logging (if enabled)
         if (LOG.isTraceEnabled()) {
             final StringBuilder sb = new StringBuilder("  setProperty(");
-            sb.append(bean);
-            sb.append(", ");
-            sb.append(name);
-            sb.append(", ");
-            if (value == null) {
-                sb.append("<NULL>");
-            } else if (value instanceof String) {
-                sb.append((String) value);
-            } else if (value instanceof String[]) {
-                final String[] values = (String[]) value;
-                sb.append('[');
-                for (int i = 0; i < values.length; i++) {
-                    if (i > 0) {
-                        sb.append(',');
-                    }
-                    sb.append(values[i]);
-                }
-                sb.append(']');
-            } else {
-                sb.append(value.toString());
-            }
-            sb.append(')');
-            LOG.trace(sb.toString());
+            LOG.trace(traceLogRecord(bean, name, value, sb).toString());;

Review Comment:
   Too many semicolons;;



##########
src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java:
##########
@@ -1067,4 +1023,57 @@ public void setProperty(final Object bean, String name, 
final Object value)
                 (e, "Cannot set " + propName);
         }
     }
+
+    /**
+     * <p>Build the stringBuilder by using set/copy bean property for log, only
+     * run when log level is <b>trace</b>. Sequentially fill stringBuilder by
+     * {@code bean.toString}, property name and property value.</p>
+     *
+     * <p><strong>When the bean's toString method is override, hide the detail
+     * of value。</strong></p>
+     *
+     * @param bean Bean on which setting is to be performed
+     * @param name Property name (can be nested/indexed/mapped/combo)
+     * @param value Value to be set
+     */
+    protected static StringBuilder traceLogRecord(Object bean, String name, 
Object value, StringBuilder sb) {

Review Comment:
   Missing `@param` tag.
   Missing `@return` tag.



##########
src/main/java/org/apache/commons/beanutils2/locale/LocaleBeanUtilsBean.java:
##########
@@ -796,32 +796,7 @@ public void setProperty(
         // Trace logging (if enabled)
         if (LOG.isTraceEnabled()) {
             final StringBuilder sb = new StringBuilder("  setProperty(");
-            sb.append(bean);
-            sb.append(", ");
-            sb.append(name);
-            sb.append(", ");
-            if (value == null) {
-                sb.append("<NULL>");
-            }
-            else if (value instanceof String) {
-                sb.append((String) value);
-            }
-            else if (value instanceof String[]) {
-                final String[] values = (String[]) value;
-                sb.append('[');
-                for (int i = 0; i < values.length; i++) {
-                    if (i > 0) {
-                        sb.append(',');
-                    }
-                    sb.append(values[i]);
-                }
-                sb.append(']');
-            }
-            else {
-                sb.append(value.toString());
-            }
-            sb.append(')');
-            LOG.trace(sb.toString());
+            LOG.trace(traceLogRecord(bean, name, value, sb).toString());;

Review Comment:
   Too many semicolons;;



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