commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gabriel Belingueres (JIRA)" <j...@apache.org>
Subject [jira] Commented: (BEANUTILS-185) [beanutils] Try to align BeanUtils with JSTL standard tags
Date Sun, 11 Jun 2006 04:12:30 GMT
    [ http://issues.apache.org/jira/browse/BEANUTILS-185?page=comments#action_12415729 ] 

Gabriel Belingueres commented on BEANUTILS-185:
-----------------------------------------------

<snip>
OK but I don't see anything different here that changes my mind about what I said about the
existing createDynaBean() in RowSetDynaClass - your new direct/indirect DynaClass implementations
could extend RowSetDynaClass and override that method. 
</snip>

Of course you can extend RowSetDynaClass directly to override the createDynaBean() method.
If you decide both to do this and decide to keep the createDynaBean() method, I guess you
should add a remark in the javadocs to show that the "best practice" for creating a different
kind of DynaBean for the RowSet is by overriding createDynaBean() method. This should be sufficient
guide for everyone.

<snip>
RowSetDynaClass is still the DynaClass (i.e. property meta data) for the individual "row"
DynaBeans (in this case BasicDynaBean) and so I would disagree with you - newInstance() is
exactly what should be called. Just because it bizarely creates a List of those DynaBeans
as well doesn't change that. 
</snip>

If this work as you said, then I guess the class name RowSetDynaClass is very confusing/misleading,
because when you call newInstance, it creates an instance of a single row of a RowSet, not
the whole RowSet. A better name would be RowDynaBean?
In addition, I see a problem here because creating a new instance make sense ONLY when iterating
over the java.sql.ResultSet; i.e. only make sense calling it from the inside, never from the
outside (the ResultSet could be already closed). Now, the newInstance() is public, and createDynaBean()
is protected...this also makes sense to me.

Perhaps the best course of action would be to separate the two concepts of RowSet and the
Row itself? We know that the DynaBean concept fits very well with the Row concept, but the
RowSet I see it more like an utility class to create the list of Rows and then I can discard
it whenever I want (usually in my applications I'm interested in the dynabean list, not the
RowSetDynaClass per se)

<snip>
Also I'm happy to add getMap() method to BasicDynaBean which returns a Map decorator - which
would provide the "indirect" access method of your requirement. 
</snip>

Don't know if this is a good idea and I suppose we think different here. 
>From a performance point of view, you probably want to add a boolean isReadonly flag as
a parameter in the BasicDynaBean constructor so that you can add a private Map instance variable
representing the unmodifiable map of properties (if isReadonly==true, or the DynaBeanMapDecorator
if false), so that whenever any of the dynabean clients call the getMap() method, only the
first time is the unmodifiable map created. (This performance optimization is probably to
make a difference if you think that the dynabean will be referenced from several JSTL expressions
in the same JSP page.)

Now if my particular application never have to deal with this new getMap() functionality:
Why would it have to pay for this new functionality/overhead on the BasicDynaBean?
May be I'm over complicating things...don't know what is the policy here in this cases.

Gabriel

> [beanutils] Try to align BeanUtils with JSTL standard tags
> ----------------------------------------------------------
>
>          Key: BEANUTILS-185
>          URL: http://issues.apache.org/jira/browse/BEANUTILS-185
>      Project: Commons BeanUtils
>         Type: Improvement

>  Environment: Operating System: other
> Platform: Other
>     Reporter: Gabriel Belingueres
>     Priority: Minor
>  Attachments: AbstractRowSetDynaClass.java, BasicDirectAccessDynaBean.java, BasicDirectAccessDynaBeanTestCase.java,
BasicIndirectAccessDynaBean.java, DirectAccessDynaBean.java, DirectRowSetDynaClass.java, DynaBeanMapDecorator.java,
DynaBeanMapDecoratorTestCase.java, IndirectAccessDynaBean.java, IndirectRowSetDynaClass.java,
beanutil-diff.txt
>
> Hi,
> I've done some modifications to the beanutils package to better support the use 
> of DynaBeans with the JSTL tags. Some of the changes are discussed in this 
> thread of the commons-user mailing list:
> http://marc.theaimsgroup.com/?l=jakarta-commons-user&m=114669123403779&w=2
> I attach the diff file that comprises changes to the RowSetDynaClass.java file 
> and build.xml file (since I added a TestCase.) Note: Please try to filter 
> carefully the diffs in the build.xml file since they include some local 
> settings I have for compilation on my machine. :-(
> Together with the diff file, I attach the new java files added to the package.
> Regards,
> Gabriel Belingueres
> gaby@ieee.org

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
   http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
   http://www.atlassian.com/software/jira


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message