On 27 Sep 2004, at 09:35, Simon Kitching wrote:
On Mon, 2004-09-27 at 18:51, Simon Kitching wrote:A few days ago I proposed adding a comment to the SetPropertiesRule file
summarising the results of the email discussion we had on this list.
Hmm... I've just (re)discovered three places in the existing code where we check whether a bean has a property or not; they all do it the same way (but different from the code just added to SetPropertiesRule).
BeanPropertySetterRule, SetPropertyRule and SetNestedPropertiesRule all use identical code as follows:
<code> // Force an exception if the property does not exist // (BeanUtils.setProperty() silently returns in this case) if (top instanceof DynaBean) { DynaProperty desc = ((DynaBean) top).getDynaClass().getDynaProperty(property); if (desc == null) { throw new NoSuchMethodException ("Bean has no property named " + property); } } else /* this is a standard JavaBean */ { PropertyDescriptor desc = PropertyUtils.getPropertyDescriptor(top, property); if (desc == null) { throw new NoSuchMethodException ("Bean has no property named " + property); } }
// Set the property (with conversion as necessary) BeanUtils.setProperty(top, property, bodyText); </code>
I've compared the code above to the PropertyUtils.isWriteable() method, and found them very very similar.
The only difference is that the above code only checks for the *existence* of a property, and will then let BeanUtils.setProperty fail silently if it is not writeable whereas PropertyUtils.isWriteable will return false (and thereby cause an error to be reported) if there *is* such a property but it is read-only.
Personally, I think that PropertyUtils.isWriteable() is the right method
to be calling; we *do* really want to raise an alert if the property
isn't writeable.
However changing the code in BeanPropertySetterRule etc is probably not advisable for compatibility reasons.
So my opinion is that we don't need to do anything; the new check in SetPropertiesRule *should* use PropertyUtils.isWriteable instead of copying the code from BeanPropertySettterRule/SetPropertyRule because it's "the right thing to do". And we should leave the (slightly broken) code in BeanPropertySetterRule etc. alone to avoid the possibility of breakage (well, I might add a warning comment).
+1
thanks for raising the original issue and investigating it. i'm not really sure that it's really solved but i'm glad that the code committed before isn't positively harmful. i'd like to think that we'll come back to this issue but maybe it's more productive to look forward...
- robert
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
