garydgregory commented on code in PR #276:
URL: https://github.com/apache/commons-beanutils/pull/276#discussion_r1742096289
##########
src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java:
##########
@@ -172,6 +172,22 @@ public BeanUtilsBean(final ConvertUtilsBean
convertUtilsBean,
this.propertyUtilsBean = propertyUtilsBean;
}
+ /**
+ * <p>Constructs an instance using given property, conversion instances
and log for logging.</p>
+ *
+ * @param convertUtilsBean use this {@code ConvertUtilsBean}
+ * to perform conversions from one object to another
+ * @param propertyUtilsBean use this {@code PropertyUtilsBean}
+ * to access properties
+ * @param log use this {@code Log}
+ * to appoint logging
+ */
+ public BeanUtilsBean(final ConvertUtilsBean convertUtilsBean, final
PropertyUtilsBean propertyUtilsBean, final Log log) {
+ this.convertUtilsBean = convertUtilsBean;
+ this.propertyUtilsBean = propertyUtilsBean;
+ LOG = log;
Review Comment:
This looks very bad IMO: Every time an instance is created, a global
variable is edited, which sounds to me like a recipe for bugs and libraries
blaming each other.
##########
src/main/java/org/apache/commons/beanutils2/BeanUtilsBean.java:
##########
@@ -1067,4 +1039,60 @@ 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
Review Comment:
This comment is not quite right IMO: It mentions logging but the method does
not log or check log levels. Since it is a protected method, it can be called
from a subclass so you have no idea how it will be used. All of this to say, I
think the Javadoc should just say what it does.
--
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]
