Robert
Thanks for taking the time to look at this and giving feedback. I take the
hint about the test cases - I'll have to get into how its done in jakarta
some time.
On point 3, I agree about fixing the exception handling in the right place,
rather than in this specific instance. The other point about this change was
a performance improvement. When the WrapDyanClass is constructed it
introspects to discover and store the PropertyDescriptors for the bean, but
WrapDyanBean uses the get/setSimpleProperty methods in PropertyUtilsBean
which means this introspection is repeated for every get/set. This seems a
shame and must be faster just to introspect once . Maybe the answer is to
split PropertyUtilsBean get/setSimpleProperty methods each into two methods
so that WrapDyanBean could take advantage of the stored PropertyDescriptors
while leaving PropertyUtilsBean processing unchanged. I include example
setSimpleProperty methods below.
Anyway 2 out of 3 would be a good result for me, I'd rather use as close to
a "vanilla" flavor of beanutils as possible - so anthing that gets into the
code base is great.
Niall
=========>Example:
public void setSimpleProperty(Object bean,
String name, Object value)
throws IllegalAccessException, InvocationTargetException,
NoSuchMethodException {
setSimpleProperty(bean, name, value, null);
}
public void setSimpleProperty(Object bean,
String name, Object value,
PropertyDescriptor propertyDescriptor)
throws IllegalAccessException, InvocationTargetException,
NoSuchMethodException {
if (bean == null) {
throw new IllegalArgumentException("No bean specified");
}
if (name == null) {
throw new IllegalArgumentException("No name specified");
}
// Validate the syntax of the property name
if (name.indexOf(PropertyUtils.NESTED_DELIM) >= 0) {
throw new IllegalArgumentException
("Nested property names are not allowed");
} else if (name.indexOf(PropertyUtils.INDEXED_DELIM) >= 0) {
throw new IllegalArgumentException
("Indexed property names are not allowed");
} else if (name.indexOf(PropertyUtils.MAPPED_DELIM) >= 0) {
throw new IllegalArgumentException
("Mapped property names are not allowed");
}
// Handle DynaBean instances specially
if (bean instanceof DynaBean) {
DynaProperty descriptor =
((DynaBean) bean).getDynaClass().getDynaProperty(name);
if (descriptor == null) {
throw new NoSuchMethodException("Unknown property '" +
name + "'");
}
((DynaBean) bean).set(name, value);
return;
}
// Retrieve the property setter method for the specified property
PropertyDescriptor descriptor = propertyDescriptor == null ?
getPropertyDescriptor(bean, name) : propertyDescriptor;
if (descriptor == null) {
throw new NoSuchMethodException("Unknown property '" +
name + "'");
}
Method writeMethod = getWriteMethod(descriptor);
if (writeMethod == null) {
throw new NoSuchMethodException("Property '" + name +
"' has no setter method");
}
// Call the property setter method
Object values[] = new Object[1];
values[0] = value;
if (log.isTraceEnabled()) {
String valueClassName =
value == null ? "<null>" : value.getClass().getName();
log.trace("setSimpleProperty: Invoking method " + writeMethod
+ " with value " + value + " (class " + valueClassName
+ ")");
}
invokeMethod(writeMethod, bean, values);
}
----- Original Message -----
From: "robert burrell donkin" <[EMAIL PROTECTED]>
To: "Jakarta Commons Developers List" <[EMAIL PROTECTED]>
Sent: Tuesday, January 06, 2004 8:32 PM
Subject: Re: [BeanUtils] WrapDynaBean Enhacement Request
> hi niall
>
> patches are much more likely to get promptly applied if they come with
> unit tests (hint, hint ;)
>
> 1+2 it seemed clear that craig intended no constructors to be available
> (and he usually has good reasons for his design decisions) but i think
> that allowing WrapDynaClass and WrapDynaBean to fit better into
> DynaBean frameworks seems like a strong argument for providing this
> functionality. unless someone else speaks up, i'll likely commit
> something along these lines.
>
> 3 seems to me to be a symptom of a bigger issue (which has been known
> for some time). the exception handling in beanutils is painful and
> confusing to many users. i'd be very reluctant to break backwards
> compatibility (even for symantics) and i suspect that the proposed
> patch does. we've talked before about the possibility of factoring out
> the exception handling and possible it might be better to fix this
> rather than the symptom. so maybe a little more talk and thought is
> needed about this one.
>
> comments welcome
>
> - robert
>
> On 6 Jan 2004, at 00:17, Niall Pemberton wrote:
>
> > Thanks, feedback would be much appreciated.
> >
> > Niall
> > ----- Original Message -----
> > From: "robert burrell donkin" <[EMAIL PROTECTED]>
> > To: "Jakarta Commons Developers List" <[EMAIL PROTECTED]>
> > Sent: Monday, January 05, 2004 10:44 PM
> > Subject: Re: [BeanUtils] WrapDynaBean Enhacement Request
> >
> >
> >> hi niall
> >>
> >> i'll take a look at this sometime soonish.
> >>
> >> - robert
> >>
> >> On 30 Dec 2003, at 13:14, Niall Pemberton wrote:
> >>
> >>> I submitted a bugzilla a while ago to enhace
> >>> WrapDynaBean/WrapDynaClass:
> >>>
> >>> http://nagoya.apache.org/bugzilla/show_bug.cgi?id=23690
> >>>
> >>>
> >>> The changes were
> >>>
> >>> 1) Implement a getInstance() method in WrapDynaBean so that the
> >>> "original" wrapped bean can be retrieved.
> >>> 2) Implement a newInstance() method in WrapDynaClass to generate new
> >>> "wrapped" beans.
> >>> 3) Change the get(name) and set(name, value) methods to make them
> >>> more
> >>> efficient and provide more usefull exception messages.
> >>>
> >>>
> >>> Is there any chance on getting some feedback and/or indication
> >>> whether
> >>> there is any interest in applying it?
> >>>
> >>> Thanks
> >>>
> >>> Niall
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: [EMAIL PROTECTED]
> >> For additional commands, e-mail: [EMAIL PROTECTED]
> >>
> >>
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [EMAIL PROTECTED]
> > For additional commands, e-mail: [EMAIL PROTECTED]
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
