struts-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ate Douma" <...@iwise1.demon.nl>
Subject Re: Duplicate Code
Date Wed, 20 Sep 2000 02:50:13 GMT
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.

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.

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.

Ate Douma
iWise BV
ate.douma@iwise.nl

----- Original Message -----
From: "Craig R. McClanahan" <Craig.McClanahan@eng.sun.com>
To: <struts-dev@jakarta.apache.org>
Sent: Wednesday, September 20, 2000 2:41 AM
Subject: Re: Duplicate Code


> Curt Hagenlocher wrote:
>
> > I noticed that there's some code duplication between
> > org.apache.struts.util.PropertyUtils and
org.apache.struts.util.BeanUtils.
> > Is this intentional?  Is one of these more "right" than the other, if I
> > want to reuse this code in my own project?
> >
>
> Sharp eyes!
>
> There is some redundancy at the moment because I am part way through a
> separation of the code that deals strictly with Java reflection
> (PropertyUtils) and the code that will deal with conversion to and from
> request parameters (BeanUtils).  At the moment, PropertyUtils is looking
> pretty solid -- BeanUtils will shortly be shedding the methods that are
> duplications, which should be completed in time for the 1.0 release I'd
like
> to have by ApacheCon Europe (Oct 23-25).
>
> If there are particular methods in BeanUtils you are planning to use let
me
> know and I can warn of potential problems.
>
> >
> > --
> > Curt Hagenlocher
> > curth@motek.com
>
> 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