struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Craig R. McClanahan" <Craig.McClana...@eng.sun.com>
Subject Re: Duplicate Code
Date Thu, 21 Sep 2000 05:40:05 GMT
See intermixed below.

Ate Douma wrote:

> Craig,
>
> Maybe you've already encountered them while refactoring, but I found two
> bugs
> in org.apache.struts.util.PropertyUtils both related to indexed properties:
> 1. In method Class getPropertyType( Object, String )
>     the return value of the getPropertyType() method of a looked up
> PropertyDescriptor
>     is returned.
>     But... for an indexed property (thus: having a
> IndexedPropertyDescriptor) this method
>     will return null if not a non indexed property accessor exists.
>     A simple patch I've implemented is:
>    <<< snip >>>
>        if ( descriptor instanceof IndexedPropertyDescriptor )
>          return
> (((IndexedPropertyDescriptor)descriptor).getIndexedPropertyType());
>        else
>          return (descriptor.getPropertyType());
>    <<< end snip >>>
> 2. In method void setIndexedProperty(Object, String, int, Object)
>     first an attempt is made to invoke the indexedWriteMethod of an indexed
> property.
>     But... after that succeeds, a return statement is missing, and instead
> the method
>     tries to update the indexed property by first retieving its Array and
> then setting the
>     indexed element within.
>     This works if an Array readMethod is defined (although setting the value
> is done twice then)
>     but results in an exception if its not.
>     Simple fix: add an return statement after the invokation of the
> writeMethod.
>

I have patched PropertyUtils as you described.  Thanks!

>
> Besides the above bugs in PropertyUtils I've encountered some problems with
> the BeanUtils class.
>
> The populate method will try to convert the request parameters into the
> correct (class) type of the bean property.
> When an empty request parameter value is submitted, the convert<Class>
> methods will always return a default initialized
> <Class> instance, e.g. Long(0), Float(0.0) etc.
> This is fine, and even required for bean properties of native java
> datatypes, but for class wrapper types this is not really
> needed and produced in our application a lot of unexplainable errors at
> first.
> When an user clears an input item on a html form we prefer to receive this
> information as a null value, as a 0 (zero) or false value
> could have another meaning altogether in certain situations (and to support
> this all our ActionForm bean properties are never of native
> datatype; if needed, at least not the setter methods which can handle null
> parameters appropriately).
>
> Thus, I've made an major change in all of the convert<Class> methods:
> 1. I've added an additional parameter boolean nullAllowed:
>     If the specified value is empty and nullAllowed, return null.
>     If the specified value is not empty but an NumberFormatException is
> encountered, return null.
>     Otherwise, the original convertion logic is used.
> 2. In all calls to the convert<Class> methods I've specified the appropriate
> boolean parameter depending on the property type.
>
> If the reasoning for doing so makes any sense, do you think these changes
> could be encorporated in later versions of struts as we really depend on
> them.
> I know this can break code that already depends on the default value
> initialization for Class type properties, but maybe an configuration
> parameter can
> be given which enables this type of behavior conditionally.
>

I can see the reasoning that led you to this approach.  A way to implement it
that doesn't break backwards compatibility would be to have two versions of each
convert method -- one with and one without the nullAllowed parameter.  That way,
you could have either kind of semantics.  Does that make sense?

>
> Finally, I've changed the populate method to make use of the PropertyUtils
> class methods as we needed the nested and indexed properties in our
> ActionForms.
> This was quite a hack, but right now it works great and we are able to
> modify complex spreadsheet/matrix tables with reconfigurable row definitions
> etc. all through
> quite simple form definitions and generic dynamic matrix ActionForm beans.
> If you whould be interested in this (a little bit clumsy but effective)
> hack, please let me know.
>

Interesting timing ... finishing up BeanUtils was on my list of stuff to do this
week.  If you wanted to send me your version of BeanUtils in private mail, I
could try to integrate it in a way compatible with what you are doing with it.

Also, if you cared to create one, I think lots of people would be interested in
a brief description of how you deal with spreadsheet/matrix type tables.

>
> Ate Douma
> iWise BV
> ate.douma@iwise.nl
>

Craig

====================
See you at ApacheCon Europe <http://www.apachecon.com>!
Session VS01 (23-Oct 13h00-17h00):  Sun Technical Briefing
Session T06  (24-Oct 14h00-15h00):  Migrating Apache JServ
                                    Applications to Tomcat



Mime
View raw message