Coder's Block

4 Opinions have been expressed on “How to Construct Clones of Non-Clonable Objects Or Construct Domain Objects Dynamically from Struts Forms”. What is your opinion?

1. Guus Bloemsma says:

   November 27, 2010 at 5:05 pm

   These methods rely on reflection to copy all properties. This means they circumvent all business logic with all the consequences that can have.

   I understand that sometimes such a hack is necessary, but for maintainability’s sake you are much better off introducing a copy constructor.

   Reply
   • Gunith says:

     November 27, 2010 at 6:15 pm

     Thanks adding your very valid point of view.. It is very true that we would have customisations when we copy properties from one object to the other. If a Bean has 10 attributes, the idea is to do all 10 in one go using this method and thereafter set the odd 3 attributes using setters and custom validations and type conversions

     Reply
2. Martin says:

   December 2, 2010 at 1:53 pm

   This approach only works well for “simple” properties. It becomes questionable for collection properties, as you have to build a “shadow” copy of the business object in the ActionForm. In any case you to expose detailed knowledge of the structure of your business objects to the view layer.
   And you circumvent any access- and validity control that might need to be applied in your business logic (as Guus already said).
   Under these circumstances you could fill the business objects themselves in the ActionForm (which you should really not do), avoiding the #copyProperties step alltogether.

   Reply
   • Gunith says:

     December 4, 2010 at 8:30 am

     Hi Martin, Thanks to you for adding your valid point of view.

     I actually tested your theory to test if the code above creates a security problem. Here are the details.

     I made 2 classes, one the bean class the other the testing class.
     The Bean Class >>

     package gunith.evillab;

     public class SampleBean {
     private boolean noGetter;
     private boolean getterSetter;

     /**
     * @param noGetter the noGetter to set
     */
     public void setNoGetter(boolean noGetter) {
     this.noGetter = noGetter;
     }

     /**
     * @return the getterSetter
     */
     public boolean isGetterSetter() {
     return getterSetter;
     }

     /**
     * @param getterSetter the getterSetter to set
     */
     public void setGetterSetter(boolean getterSetter) {
     this.getterSetter = getterSetter;
     }

     /* (non-Javadoc)
     * @see java.lang.Object#toString()
     */

     @Override
public String toString() {
StringBuilder builder = new StringBuilder();
builder.append("SampleBean [noGetter=");
builder.append(noGetter);
builder.append(", getterSetter=");
builder.append(getterSetter);
builder.append("]");
return builder.toString();
}
}

     Then the test class >>

     package gunith.evillab;
     
import java.lang.reflect.InvocationTargetException;
import org.apache.commons.beanutils.BeanUtilsBean;

     public class SampleBeanCopier {

     public static void main(String[] args) {
     SampleBean src = new SampleBean();
     SampleBean dest = new SampleBean();

     src.setGetterSetter(true);
     src.setNoGetter(true);

     BeanUtilsBean bub = new BeanUtilsBean();

     try {
bub.copyProperties(dest, src);
}
catch (IllegalAccessException e) {
e.printStackTrace();
}
catch (InvocationTargetException e) {
e.printStackTrace();
}
System.out.println(dest.toString());
}
}

     Well, the result is that the boolean without the getter (noGetter) has false while getterSetter returns true. That shows that class the BeanUtilBeans uses getters and setters instead of the private properties.

     As for the one about the ‘shadow collection’ theory I can’t figure out sure how to test. We can’t do a hashCode test, can we?

     Reply