RE: DynaActionForm.getMap() change
Craig, Just got back from Holidays. Am I already late to party? How about `DynaActionForm' with unmodifiable map `MutableDynaActionForm' with a direct modifiable map. -- Peter Pilgrim, Struts/J2EE Consultant, RBoS FM, Risk IT Tel: +44 (0)207-375-4923 -Original Message- From: Craig R. McClanahan [mailto:[EMAIL PROTECTED] Sent: 16 July 2003 20:30 To: Struts Developers List Subject: Re: DynaActionForm.getMap() change On Wed, 16 Jul 2003, Rob Leland wrote: Date: Wed, 16 Jul 2003 14:58:45 -0400 From: Rob Leland [EMAIL PROTECTED] Reply-To: Struts Developers List [EMAIL PROTECTED] To: Struts Developers List [EMAIL PROTECTED] Subject: Re: DynaActionForm.getMap() change David Graham wrote: DynaActionForm.getMap() returns the Map that the properties are stored in without copying it to a new Map instance or wrapping it in an unmodifiable Map. IMO, we should treat that method with much more care and return an unmodifiable view of the Map considering it's mainly to be used in JSTL tags. Does this solve a problem that you or others have encountered in practice ? I can see where a use who retrieves the Map and then modifies it could accidentally (or on purpose) violate the type safety checks that the normal get() and set() calls provide for you. -Rob Is anyone opposed to this idea? My only concern would be the performance impact of this, especially if it happens more than once in the same page (say, because you used the same dynabean in more than one EL expression). An alternative approach would be to do what someone else suggested and make DynaActionForm implement Map directly -- but make it effectively immutable by throwing UnsupportedOperationException for Map method calls like put() and remove(). I don't *think* there are any method signature overlaps that would cause us any grief. David Craig - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] *** This e-mail is intended only for the addressee named above. As this e-mail may contain confidential or privileged information, if you are not the named addressee, you are not authorised to retain, read, copy or disseminate this message or any part of it. The Royal Bank of Scotland plc is registered in Scotland No 90312 Registered Office: 36 St Andrew Square, Edinburgh EH2 2YB Regulated by the Financial Services Authority Visit our website at http://www.rbs.co.uk/CBFM/ *** - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DynaActionForm.getMap() change
I don't like the performance impact of creating a new wrapper instance on every call to getMap(), however -- that can happen lots of times in a page. It's probably possible to create the wrapper once and then cache it (to reduce the impact somewhat), but it is still a spurious object creation. I considered caching as well but thought the implementation might get a bit ugly. It also doesn't help the form.map.prop issue. I have an idea that avoids caching and spurious object creation, but does not fix the form.map.prop issue. The idea is to create a new FixedKeyMap class which delegates all methods and constructors to the HashMap class with the following exceptions: - put(), putAll(), remove(), and clear() throw UnsupportedOperationException - a new set method with the same signature as the put method is created, but it can only be used to override values of existing keys. So the end effect is that when the map is created the key set is fixed forever by the constructor. The map's values can change, but its keys cannot. It's ugly for a Java or JSP developer to change the bean's properties using the Map instead of the methods in the DynaActionForm class, but at least it shouldn't cause any strange exceptions to occur. I'm just throwing this idea out there... actually I prefer having DynaActionForm implement Map directly. Matt - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
DynaActionForm.getMap() change
DynaActionForm.getMap() returns the Map that the properties are stored in without copying it to a new Map instance or wrapping it in an unmodifiable Map. IMO, we should treat that method with much more care and return an unmodifiable view of the Map considering it's mainly to be used in JSTL tags. Is anyone opposed to this idea? David __ Do you Yahoo!? SBC Yahoo! DSL - Now only $29.95 per month! http://sbc.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DynaActionForm.getMap() change
David Graham wrote: DynaActionForm.getMap() returns the Map that the properties are stored in without copying it to a new Map instance or wrapping it in an unmodifiable Map. IMO, we should treat that method with much more care and return an unmodifiable view of the Map considering it's mainly to be used in JSTL tags. Does this solve a problem that you or others have encountered in practice ? -Rob Is anyone opposed to this idea? David - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DynaActionForm.getMap() change
--- Craig R. McClanahan [EMAIL PROTECTED] wrote: On Wed, 16 Jul 2003, Rob Leland wrote: Date: Wed, 16 Jul 2003 14:58:45 -0400 From: Rob Leland [EMAIL PROTECTED] Reply-To: Struts Developers List [EMAIL PROTECTED] To: Struts Developers List [EMAIL PROTECTED] Subject: Re: DynaActionForm.getMap() change David Graham wrote: DynaActionForm.getMap() returns the Map that the properties are stored in without copying it to a new Map instance or wrapping it in an unmodifiable Map. IMO, we should treat that method with much more care and return an unmodifiable view of the Map considering it's mainly to be used in JSTL tags. Does this solve a problem that you or others have encountered in practice ? I can see where a use who retrieves the Map and then modifies it could accidentally (or on purpose) violate the type safety checks that the normal get() and set() calls provide for you. -Rob Is anyone opposed to this idea? My only concern would be the performance impact of this, especially if it happens more than once in the same page (say, because you used the same dynabean in more than one EL expression). That's why I suggested the unmodifiable wrapper technique instead of copying to a new Map. But, now that I think about it, a new wrapper class would be created each time you referenced the map which is a potential performance issue. An alternative approach would be to do what someone else suggested and make DynaActionForm implement Map directly -- but make it effectively immutable by throwing UnsupportedOperationException for Map method calls like put() and remove(). I don't *think* there are any method signature overlaps that would cause us any grief. I'm not convinced yet that cramming a DynaActionForm into the Map interface is appropriate. It seems like a hack. Just because it uses a Map for its implementation doesn't mean that the bean itself should be a Map. David Craig - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] __ Do you Yahoo!? SBC Yahoo! DSL - Now only $29.95 per month! http://sbc.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: DynaActionForm.getMap() change
--- Rob Leland [EMAIL PROTECTED] wrote: David Graham wrote: DynaActionForm.getMap() returns the Map that the properties are stored in without copying it to a new Map instance or wrapping it in an unmodifiable Map. IMO, we should treat that method with much more care and return an unmodifiable view of the Map considering it's mainly to be used in JSTL tags. Does this solve a problem that you or others have encountered in practice ? The problem is that it violates OOP rules of encapsulation. Whether or not I've personally had a problem with this is irrelevant. We should be trying to stop bugs before they're reported and Craig pointed out a very possible way of getting into trouble with the current functionality. I'm not sure about acceptable solutions but the point was to raise the issue and get feedback. David -Rob Is anyone opposed to this idea? David - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] __ Do you Yahoo!? SBC Yahoo! DSL - Now only $29.95 per month! http://sbc.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
RE: DynaActionForm.getMap() change
My 0.2 cents - I think, idea to provide getMap() method is great ! Yes, to solve the performance problem, I think, then We will have to keep track when underlying map has Changed. Then, create unmodifiable map only if it changed Since the last time... Actually, this is a common java idiom, I think. Whenever We need to expose a collection from a class, we should return unmodifiable variant of it so that encapsulation doesn't break, but then, we have performance issue, as David said... Thanks. -Original Message- From: David Graham [mailto:[EMAIL PROTECTED] Sent: Wednesday, July 16, 2003 12:46 PM To: Struts Developers List Subject: Re: DynaActionForm.getMap() change --- Craig R. McClanahan [EMAIL PROTECTED] wrote: On Wed, 16 Jul 2003, Rob Leland wrote: Date: Wed, 16 Jul 2003 14:58:45 -0400 From: Rob Leland [EMAIL PROTECTED] To: Struts Developers List [EMAIL PROTECTED] Subject: Re: DynaActionForm.getMap() change David Graham wrote: DynaActionForm.getMap() returns the Map that the properties are stored in without copying it to a new Map instance or wrapping it in an unmodifiable Map. IMO, we should treat that method with much more care and return an unmodifiable view of the Map considering it's mainly to be used in JSTL tags. Does this solve a problem that you or others have encountered in practice ? I can see where a use who retrieves the Map and then modifies it could accidentally (or on purpose) violate the type safety checks that the normal get() and set() calls provide for you. -Rob Is anyone opposed to this idea? My only concern would be the performance impact of this, especially if it happens more than once in the same page (say, because you used the same dynabean in more than one EL expression). That's why I suggested the unmodifiable wrapper technique instead of copying to a new Map. But, now that I think about it, a new wrapper class would be created each time you referenced the map which is a potential performance issue. An alternative approach would be to do what someone else suggested and make DynaActionForm implement Map directly -- but make it effectively immutable by throwing UnsupportedOperationException for Map method calls like put() and remove(). I don't *think* there are any method signature overlaps that would cause us any grief. I'm not convinced yet that cramming a DynaActionForm into the Map interface is appropriate. It seems like a hack. Just because it uses a Map for its implementation doesn't mean that the bean itself should be a Map. David Craig - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED] __ Do you Yahoo!? SBC Yahoo! DSL - Now only $29.95 per month! http://sbc.yahoo.com - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
RE: DynaActionForm.getMap() change
On Wed, 16 Jul 2003, Ranjangaonkar, Vaibhav (HQP) wrote:
Date: Wed, 16 Jul 2003 12:56:09 -0700
From: Ranjangaonkar, Vaibhav (HQP) [EMAIL PROTECTED]
Reply-To: Struts Developers List [EMAIL PROTECTED]
To: Struts Developers List [EMAIL PROTECTED]
Subject: RE: DynaActionForm.getMap() change
My 0.2 cents -
I think, idea to provide getMap() method is great !
Yes, to solve the performance problem, I think, then
We will have to keep track when underlying map has
Changed. Then, create unmodifiable map only if it changed
Since the last time...
The current implementation already has getMap() -- the problem is that it
returns the real HashMap that is internally used to store property values,
which exposes those values to being modified by means other than through
the DynaActionForm methods.
I don't like the performance impact of creating a new wrapper instance on
every call to getMap(), however -- that can happen lots of times in a
page. It's probably possible to create the wrapper once and then cache it
(to reduce the impact somewhat), but it is still a spurious object
creation.
Actually, this is a common java idiom, I think. Whenever
We need to expose a collection from a class, we should
return unmodifiable variant of it so that encapsulation
doesn't break, but then, we have performance issue, as
David said...
Thanks.
The reason to think about doing it is usability, which sometimes needs to
win out over O-O purity :-).
All of the Struts tags, because they use commons-beanutils under the
covers, treat DynaBean beans and standard JavaBeans identically -- the
internal implementation knows exactly what to do. So you can say things
like:
html:bean:write name=customer property=firstName/
Now, assume you've got a page author that wants to switch and use JSTL EL
expressions instead. Their natural first inclination is going to be to
write:
c:out value=${customer.firstName}/
which will work fine for a standard JavaBean, but not fine for a DynaBean.
If it *happens* to be a DynaActionForm, you can work around this by
saying:
c:out value=${customer.map.firstName}/
because we added getMap() to DynaActionForm. This is really confusing to
newcomers, and would be unnecessary if we went to DynaActionForm
implementing Map. Then, the user's initial guess at how to write the
expression would have been correct.
Given that JSP 2.0 makes EL expressions available everywhere in a page, so
that you'd be able to write just this instead:
${customer.firstName}
then I think the usability issue is strong enough that we should look at
making DynaActionForm implement Map so that it plays nicely with all EL
expressions.
SIDE NOTE: In JavaServer Faces, the APIs let you customize a thing called
the PropertyResolver, which is the piece of code that interprets the .
or [] operators in an EL expression. In the most recent release of the
Struts-Faces integration library, I took of advantage of this and
implemented support for DynaBeans -- but that only works for value
reference expressions in Faces components, not in other uses of EL in the
page.
Craig
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
RE: DynaActionForm.getMap() change
--- Craig R. McClanahan [EMAIL PROTECTED] wrote:
On Wed, 16 Jul 2003, Ranjangaonkar, Vaibhav (HQP) wrote:
Date: Wed, 16 Jul 2003 12:56:09 -0700
From: Ranjangaonkar, Vaibhav (HQP) [EMAIL PROTECTED]
Reply-To: Struts Developers List [EMAIL PROTECTED]
To: Struts Developers List [EMAIL PROTECTED]
Subject: RE: DynaActionForm.getMap() change
My 0.2 cents -
I think, idea to provide getMap() method is great !
Yes, to solve the performance problem, I think, then
We will have to keep track when underlying map has
Changed. Then, create unmodifiable map only if it changed
Since the last time...
The current implementation already has getMap() -- the problem is that
it
returns the real HashMap that is internally used to store property
values,
which exposes those values to being modified by means other than through
the DynaActionForm methods.
I don't like the performance impact of creating a new wrapper instance
on
every call to getMap(), however -- that can happen lots of times in a
page. It's probably possible to create the wrapper once and then cache
it
(to reduce the impact somewhat), but it is still a spurious object
creation.
I considered caching as well but thought the implementation might get a
bit ugly. It also doesn't help the form.map.prop issue.
Actually, this is a common java idiom, I think. Whenever
We need to expose a collection from a class, we should
return unmodifiable variant of it so that encapsulation
doesn't break, but then, we have performance issue, as
David said...
Thanks.
The reason to think about doing it is usability, which sometimes needs
to
win out over O-O purity :-).
I often argue from the OO purist perspective (if you hadn't guessed by now
:-). However, sometimes that's just playing devil's advocate. I think
the usability issue here probably justifies DynaActionForm implementing
Map. Is there any other way of acheiving form.prop for a DynaActionForm?
If we do implement Map we should note in the reasons in the javadoc so
it's less confusing for extenders/customizers.
David
All of the Struts tags, because they use commons-beanutils under the
covers, treat DynaBean beans and standard JavaBeans identically -- the
internal implementation knows exactly what to do. So you can say things
like:
html:bean:write name=customer property=firstName/
Now, assume you've got a page author that wants to switch and use JSTL
EL
expressions instead. Their natural first inclination is going to be to
write:
c:out value=${customer.firstName}/
which will work fine for a standard JavaBean, but not fine for a
DynaBean.
If it *happens* to be a DynaActionForm, you can work around this by
saying:
c:out value=${customer.map.firstName}/
because we added getMap() to DynaActionForm. This is really confusing
to
newcomers, and would be unnecessary if we went to DynaActionForm
implementing Map. Then, the user's initial guess at how to write the
expression would have been correct.
Given that JSP 2.0 makes EL expressions available everywhere in a page,
so
that you'd be able to write just this instead:
${customer.firstName}
then I think the usability issue is strong enough that we should look at
making DynaActionForm implement Map so that it plays nicely with all EL
expressions.
SIDE NOTE: In JavaServer Faces, the APIs let you customize a thing
called
the PropertyResolver, which is the piece of code that interprets the .
or [] operators in an EL expression. In the most recent release of
the
Struts-Faces integration library, I took of advantage of this and
implemented support for DynaBeans -- but that only works for value
reference expressions in Faces components, not in other uses of EL in
the
page.
Craig
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
__
Do you Yahoo!?
SBC Yahoo! DSL - Now only $29.95 per month!
http://sbc.yahoo.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: DynaActionForm.getMap() change
i don't know about JSTL-EL, but Velocity users would only need a
get(String
key) method to have ${form.prop} work for us. and i think this will
make
several of us happy too! :)
Because DynaActionForm is a DynaBean, it already has a get(String) method.
David
Nathan Bubna
[EMAIL PROTECTED]
__
Do you Yahoo!?
SBC Yahoo! DSL - Now only $29.95 per month!
http://sbc.yahoo.com
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: DynaActionForm.getMap() change
David Graham said:
i don't know about JSTL-EL, but Velocity users would only need a
get(String
key) method to have ${form.prop} work for us. and i think this will
make
several of us happy too! :)
Because DynaActionForm is a DynaBean, it already has a get(String) method.
cool. i was happy and didn't even know it! thx! :)
Nathan Bubna
[EMAIL PROTECTED]
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
Re: DynaActionForm.getMap() change
On Wed, 16 Jul 2003, David Graham wrote:
Date: Wed, 16 Jul 2003 17:35:10 -0700 (PDT)
From: David Graham [EMAIL PROTECTED]
Reply-To: Struts Developers List [EMAIL PROTECTED],
[EMAIL PROTECTED]
To: Struts Developers List [EMAIL PROTECTED]
Subject: Re: DynaActionForm.getMap() change
i don't know about JSTL-EL, but Velocity users would only need a
get(String
key) method to have ${form.prop} work for us. and i think this will
make
several of us happy too! :)
Because DynaActionForm is a DynaBean, it already has a get(String) method.
It even returns the property value ... imagine that :-).
David
Craig
-
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
