You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@struts.apache.org by David Graham <gr...@yahoo.com> on 2003/07/16 19:30:49 UTC

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: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org


Re: DynaActionForm.getMap() change

Posted by David Graham <gr...@yahoo.com>.
--- "Craig R. McClanahan" <cr...@apache.org> wrote:
> 
> 
> On Wed, 16 Jul 2003, Rob Leland wrote:
> 
> > Date: Wed, 16 Jul 2003 14:58:45 -0400
> > From: Rob Leland <rl...@apache.org>
> > Reply-To: Struts Developers List <st...@jakarta.apache.org>
> > To: Struts Developers List <st...@jakarta.apache.org>
> > 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: struts-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: struts-dev-help@jakarta.apache.org
> 


__________________________________
Do you Yahoo!?
SBC Yahoo! DSL - Now only $29.95 per month!
http://sbc.yahoo.com

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


Re: DynaActionForm.getMap() change

Posted by "Craig R. McClanahan" <cr...@apache.org>.

On Wed, 16 Jul 2003, Rob Leland wrote:

> Date: Wed, 16 Jul 2003 14:58:45 -0400
> From: Rob Leland <rl...@apache.org>
> Reply-To: Struts Developers List <st...@jakarta.apache.org>
> To: Struts Developers List <st...@jakarta.apache.org>
> 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: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org


Re: DynaActionForm.getMap() change

Posted by David Graham <gr...@yahoo.com>.
--- Rob Leland <rl...@apache.org> 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: struts-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: struts-dev-help@jakarta.apache.org
> 


__________________________________
Do you Yahoo!?
SBC Yahoo! DSL - Now only $29.95 per month!
http://sbc.yahoo.com

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


Re: DynaActionForm.getMap() change

Posted by Rob Leland <rl...@apache.org>.
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: struts-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: struts-dev-help@jakarta.apache.org