You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Christoph Reck <Ch...@dlr.de> on 2000/12/07 18:33:51 UTC

[PROPOSAL] Allignment of introspection

Currently the ASTReference and ASTIdentifier (also ASTSetDirective)
handle set and get method hardwired and differently. 

ASTIdentifier knows the Map collection class get() method, 
whereas ASTSetDirective via ASTReference.setValue(...) does not 
know how to handle the set method of the Map interface.

An access to $list.size without () throws a ClassCastException
whereas "#set $list.Size = 10" works (because there is a 
setSize method).

Proposal:
Allign the handling of references and indentifiers to accept
getting and setting of:
* Patch on ASTReference.setValue(...) to allow a set for 
  objects implementing the Map interface.
* Patch for ASTReference to use the introspection engine
  to allow setValue to a method with a superclass/primitive
  signature, e.g.: #set $list.Size = 10
* Patch ASTIdentifier to allow accessing method names not 
  following bean conventions (e.g. methods not prefixed with 
  get). This is already possible by just adding empty ()
  after the identifier (this makes the node into a ASTMethod!)
  - but this is not orthogonal usage: no brakets required
  for getXXX methods, but yes for XXX() methods.
* Alternatively fix ASTIdentifier/MapExecutor not to throw 
  a ClassCastException.
* Set and get object public fields.
* Add a feature to ASTReference to access methods/fields of 
  static classes.
These fixes/enhancements will have no impact on the current
mode of operation. I can also provide patches to the test 
cases ensuring these features work.

Please send votes for each point independently.

I will be able to provide the patches quickly after three +1s.

:) Christoph

Re: [PROPOSAL] Allignment of introspection

Posted by Christoph Reck <Ch...@dlr.de>.
yet this thread is not going anywhere whare I can move to using 
Velocily instead of WM in a staightforward manner.

I would prefer the syntax 
  #set( $foo.bar.crow = "wacky")
over 
  #set( $dummy = $foo.bar.set("crow", "wacky") )
  #* The dummy is required to ingore any output that would 
     emerge when replacing objects in the Map. *#
which is the last thing I need to cleanly move from WM to Vel
(apart from the bugs mentioned in another thread, which I would
either workaround or send patches to fix them).

My explicit application I have configuration and control templates 
(similar to the texen task) which setup some hastables.
I could move these to java code or XML, but I originally did
it in WM beacuse it avoided having parts spread around in 
different formats (VTL, XML, XSL, Java, etc.).
  * model is directories, files, scripts and XML - these are the
    predefined interfaces to my application (currently no CORBA, HTTP,
    SQL/ODBC, nor XMLRPC).
  * view is VTL (had XSL, but moved to VSL==VTL!)
  * control is servlet (Turbine+screen classes), Java context tools, 
    and VTL sequencing everything (<evil>decoding actions</evil>,
    reading/writing files, <devlish>calling scripts!</devlish>, 
    setting up parameters for and loading the views.
The control VTL is clearly separated from the view in my design 
(sometimes during prototyping not - ugh! JSP-like) and does finally 
a "parse $view", and views can include other views - real HMVC.

Yes I could do this the turbine-way by doing all controlling within 
the actions and screen classes; but, I like the ease of scripting 
(VTL is sufficient) and avoid having to recompile/restart everything - 
for prototyping. For production with cached AST it is then sufficiently 
fast.


So:
  + shall I prepare a patch to add the notion of Map.put() to 
    the #set directive? Yes, I will respect the caching added by
    Geir in this area.
Or:
  - Shall I live with the 
      <ugly>#set( $dummy = $foo.bar.set("crow", "wacky") )</ugly>
    form?
  - Shall we impose this limitiation to people wanting to go from
    WM to Vel (impeding automatic convert.WebMacro)?

The patch I propose is (sorry for repeating this, but I'm not getting 
any clear votes on this proposal):
  ASTSetDirective.render() - to fix a bug I have reported:
    + Assigning a NULL value to the context will remove the entry
      (since Hashmap does not allow it).
    + make it pass even null values to the LHS ASTReference.setValue()
  ASTReference.setValue() to differentiate between maps and
    methods (using the introspection engine)
    + Assigning a NULL value to a map will remove the map entry
    + possibly uppercasing the first character to allow something like
      "#set $object.size = 10" access setSize().
  ASTIdentifier.execute() get rid of the Executor classes, make 
    the doIntrospection do the inverse that ASTReference.setValue()
    does. Resulting in "Given $object.size we could search for
    $object.getSize(), $object.size(), $object.get("size")" and 
    possibly $object.size (public field).
This way setting is concentrated within ASTSetDirective (context.put)
or ASTReference.setValue() (introspection) and getting is all done 
within ASTReference (context.get) or the following ASTIdentifiers
(introspection). This will allow consistent handling.

If you give me a clear -1 on this proposal, I'll drop this thread
and do it the <ugly/> way.

:) Christoph

Re: [PROPOSAL] Allignment of introspection

Posted by jv...@periapt.com.
Hi Christoph,

I'm just looking over the list now :-)

jvz.

On Thu, 7 Dec 2000, Christoph Reck wrote:

> Currently the ASTReference and ASTIdentifier (also ASTSetDirective)
> handle set and get method hardwired and differently. 
> 
> ASTIdentifier knows the Map collection class get() method, 
> whereas ASTSetDirective via ASTReference.setValue(...) does not 
> know how to handle the set method of the Map interface.
> 
> An access to $list.size without () throws a ClassCastException
> whereas "#set $list.Size = 10" works (because there is a 
> setSize method).
> 
> Proposal:
> Allign the handling of references and indentifiers to accept
> getting and setting of:
> * Patch on ASTReference.setValue(...) to allow a set for 
>   objects implementing the Map interface.
> * Patch for ASTReference to use the introspection engine
>   to allow setValue to a method with a superclass/primitive
>   signature, e.g.: #set $list.Size = 10
> * Patch ASTIdentifier to allow accessing method names not 
>   following bean conventions (e.g. methods not prefixed with 
>   get). This is already possible by just adding empty ()
>   after the identifier (this makes the node into a ASTMethod!)
>   - but this is not orthogonal usage: no brakets required
>   for getXXX methods, but yes for XXX() methods.
> * Alternatively fix ASTIdentifier/MapExecutor not to throw 
>   a ClassCastException.
> * Set and get object public fields.
> * Add a feature to ASTReference to access methods/fields of 
>   static classes.
> These fixes/enhancements will have no impact on the current
> mode of operation. I can also provide patches to the test 
> cases ensuring these features work.
> 
> Please send votes for each point independently.
> 
> I will be able to provide the patches quickly after three +1s.
> 
> :) Christoph
> 


Re: [PROPOSAL] Allignment of introspection

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Here are some ideas to incorporate into this discussion :

I think (and have discussed this with jason, although I won't make any
claims about his agreement on this) that we might want to not be so
accomodating to users on the introspection front.  Here's what I am
thinking about, in no particular order (and some we already do,
probably) :

0) I think we should, in our introspection, try the literal
interpretation of the reference before we wander of in introspection
flail-land.  This is for speed, and we may already do this.

1) I think that we should conform to the bean spec.  There is already
much literature and support for it.

2) I think that we should clearly define and document what our
introspection plan is so a user can code templates to that for speed. 
Interested, Christoph?

3) I think we should consider limiting our introspection plan to not try
anything and everything as a convenience for the users.  Why?  Well,
consider that we aren't looking for gold or oil here - we are trying to
match a method call or data access in which a programmer and designer
have already sat down and agreed on the API they will use between them
through the context.  Of course, it's a nice feature that you can do
things like $foo.bar and the programmer has the freedom to use a method
bar(), getBar() or get("bar") in the object $foo, but does that conform
to the bean spec anyway?

4) My gut reaction is -1 to #set $foo.bar = RHS for some reason, but
will think about it. I think because this is a loaded gun of sorts,
depending upon the class.

5) [wacky]  I can't come up with a convincing example, but consider if
we allowed users, through a vel.propse property, to tell Velocity to
*only* try the first approach of the introspection plan and then abandon
the search.  For specialized cases, I wonder if there would be a good
performance gain.  It's like the user is saying "Hey, I promise to be
careful about my reference use in the template, if you (Velocity)
promise to be quick about it...".

A little more inline.

Christoph Reck wrote:
> 
> Hi,
> 
> The patch I propose is:
>   ASTSetDirective.render() - to fix a bug I have reported:
>     + Assigning a NULL value to the context will remove the entry
>       (since Hashmap does not allow it).

MMmmmm.  Are you sure that's a good idea?  Doesn't that mean something
could be GC'd if the app was depending upon the context to hold
something? Granted that would happen if the RHS wasn't nul.... hmm.


>     + make it pass even null values to the LHS ASTReference.setValue()
>   ASTReference.setValue() to differentiate between maps and
>     methods (using the introspection engine)
>     + Assigning a NULL value to a map will remove the map entry

see above

>     + possibly uppercasing the first character to allow something like
>       "#set $object.size = 10" access setSize().
>     + possibly adding the notion of getting/setting fields (if voted for)
>     + possibly: if the rootString is not in the context check with
>       the classloader if the dotted path (ASTIdentifiers) point
>       to a static method of a class... similar to the xsl:java
>       taglib. (if voted for)

Isn't that slow?  The issue would be if something starting with $ wasn't
a reference (such as alt.on.$sale.now ) how much would this slow
rendering down?

>   ASTIdentifier.execute() get rid of the Executor classes, make
>     the doIntrospection do the inverse that ASTReference.setValue()
>     does. Resulting in "Given $object.size we could search for
>     $object.getSize(), $object.size(), $object.get("size")" and
>     possibly $object.size (public field).
> This way setting is concentrated within ASTSetDirective (context.put)
> or ASTReference.setValue() (introspection) and getting is all done
> within ASTReference (context.get) or the following ASTIdentifiers
> (introspection). This will allow consistent handling.
> 
> More embedded.
> 
> jvanzyl@periapt.com wrote:
> >
> > On Thu, 7 Dec 2000, Christoph Reck wrote:
> >
> > > Currently the ASTReference and ASTIdentifier (also ASTSetDirective)
> > > handle set and get method hardwired and differently.
> >
> > They are different for reason. When setting the value of
> > reference strict bean behaviour is used, there has to
> > be a setXXX(). For properties, handled by ASTIdentifier
> > we again use bean behaviour with the extension for Maps.
> 
> OK, if the contract for #set is to stick to the bean definition,
> we'll live with it... more below...
> 
> But it seems strange that $hashtable.XXX works but
> #set $hashtable.XXX = "anything" does not!
> 
> >
> > > ASTIdentifier knows the Map collection class get() method,
> > > whereas ASTSetDirective via ASTReference.setValue(...) does not
> > > know how to handle the set method of the Map interface.
> >
> > There is no set method in the Map interface.
> 
> I meant "put", dummy me!
> 
> >
> > > An access to $list.size without () throws a ClassCastException
> > > whereas "#set $list.Size = 10" works (because there is a
> > > setSize method).
> >
> > Right, we are trying to follow standard bean properties.
> > A List is not a bean, and List.size() is not a property.
> > But we could certainly extend the search for your example
> > above. Given $object.size we could search for $object.getSize(),
> > $object.size(), $object.get("size")
> >
> > Is that what you're talking about?
> 
> exactly!
> 
> >
> > > Proposal:
> > > Allign the handling of references and indentifiers to accept
> > > getting and setting of:
> > > * Patch on ASTReference.setValue(...) to allow a set for
> > >   objects implementing the Map interface.
> >
> > So you want
> >
> > #set $map.key = $value
> >
> > instead of
> >
> > $map.put("key", $value)
> >
> > If that's the case then it's it looks pretty limiting: the
> > only thing you can use for the key are strings. I'm not
> > sure there's much benefit in changing the behaviour when
> > $map.put($this,$that) is available. But it is not hard
> > to implement, and if designers are using them then
> > consistent use of #set might be desirable.
> >
> 
> The limiting is not an impact, since we are talking of
> indentifiers. Something like #set $map.$objectKey... is out
> of scope and should not be supported.

It won't be.  it won't even parse

> 
> > > * Patch for ASTReference to use the introspection engine
> > >   to allow setValue to a method with a superclass/primitive
> > >   signature, e.g.: #set $list.Size = 10
> >
> > I agree that primitive properties should be supported,
> > but I don't agree with an object with a size(int i)
> > method being supported by #set. The #set is for setting
> > values of references (a property is a reference) not
> > executing arbitrary methods. Why not simply use
> > $list.size(1) instead of #set $list.size = 1 ?
> 
> OK, I could agree here. We can restrict set and identifiers to
> the bean pattern AND maps. Methods should do the rest...
> 
> >
> > > * Patch ASTIdentifier to allow accessing method names not
> > >   following bean conventions (e.g. methods not prefixed with
> > >   get). This is already possible by just adding empty ()
> > >   after the identifier (this makes the node into a ASTMethod!)
> > >   - but this is not orthogonal usage: no brakets required
> > >   for getXXX methods, but yes for XXX() methods.
> >
> > getXXX() are properties and XXX() are not. This was the
> > original reason, to follow bean behaviour. Many people
> > seem to want this so it could be added.
> 
> I guess people are spoiled by the WM form (and are having a hard
> time porting from it - as myself). Vel does allow the abbreviated
> form of $ref.Xxx to mean $ref.getXxx() and this misleads to
> believe more power behind the introspection...

And I think this is the time to nip that in the bud...

> If you +1 this, I'll provide the patch for the "Given $object.size
> we could search for $object.getSize(), $object.size(),
> $object.get("size")" form.
> 
> >
> > > * Alternatively fix ASTIdentifier/MapExecutor not to throw
> > >   a ClassCastException.
> > > * Set and get object public fields.
> >
> > > * Add a feature to ASTReference to access methods/fields of
> > >   static classes.
> >
> > Static methods work. I use them in Texen/Torque all
> > the time. I don't really like being able to access
> > public fields.
> 
> I meant something like #set $foo = $java.util.Arrays.sort($list)
> 
> I can agree to drop the need of accessing public fields in the
> Vel environment... If it where to access things like
> javax.swing.SwingConstants.CENTER it would be very desireable...
> 
> >
> > jvz.
> 
> I'll start as soon as at least you give me a +1 and I have JTest
> running again.
> 
> :) Christoph

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

Re: [PROPOSAL] Allignment of introspection

Posted by Christoph Reck <Ch...@dlr.de>.
Hi,

The patch I propose is:
  ASTSetDirective.render() - to fix a bug I have reported:
    + Assigning a NULL value to the context will remove the entry
      (since Hashmap does not allow it).
    + make it pass even null values to the LHS ASTReference.setValue()
  ASTReference.setValue() to differentiate between maps and
    methods (using the introspection engine)
    + Assigning a NULL value to a map will remove the map entry
    + possibly uppercasing the first character to allow something like
      "#set $object.size = 10" access setSize().
    + possibly adding the notion of getting/setting fields (if voted for)
    + possibly: if the rootString is not in the context check with 
      the classloader if the dotted path (ASTIdentifiers) point
      to a static method of a class... similar to the xsl:java 
      taglib. (if voted for)
  ASTIdentifier.execute() get rid of the Executor classes, make 
    the doIntrospection do the inverse that ASTReference.setValue()
    does. Resulting in "Given $object.size we could search for
    $object.getSize(), $object.size(), $object.get("size")" and 
    possibly $object.size (public field).
This way setting is concentrated within ASTSetDirective (context.put)
or ASTReference.setValue() (introspection) and getting is all done 
within ASTReference (context.get) or the following ASTIdentifiers
(introspection). This will allow consistent handling.

More embedded.

jvanzyl@periapt.com wrote:
> 
> On Thu, 7 Dec 2000, Christoph Reck wrote:
> 
> > Currently the ASTReference and ASTIdentifier (also ASTSetDirective)
> > handle set and get method hardwired and differently.
> 
> They are different for reason. When setting the value of
> reference strict bean behaviour is used, there has to
> be a setXXX(). For properties, handled by ASTIdentifier
> we again use bean behaviour with the extension for Maps.

OK, if the contract for #set is to stick to the bean definition, 
we'll live with it... more below...

But it seems strange that $hashtable.XXX works but 
#set $hashtable.XXX = "anything" does not!

> 
> > ASTIdentifier knows the Map collection class get() method,
> > whereas ASTSetDirective via ASTReference.setValue(...) does not
> > know how to handle the set method of the Map interface.
> 
> There is no set method in the Map interface.

I meant "put", dummy me!

> 
> > An access to $list.size without () throws a ClassCastException
> > whereas "#set $list.Size = 10" works (because there is a
> > setSize method).
> 
> Right, we are trying to follow standard bean properties.
> A List is not a bean, and List.size() is not a property.
> But we could certainly extend the search for your example
> above. Given $object.size we could search for $object.getSize(),
> $object.size(), $object.get("size")
> 
> Is that what you're talking about?

exactly!

> 
> > Proposal:
> > Allign the handling of references and indentifiers to accept
> > getting and setting of:
> > * Patch on ASTReference.setValue(...) to allow a set for
> >   objects implementing the Map interface.
> 
> So you want
> 
> #set $map.key = $value
> 
> instead of
> 
> $map.put("key", $value)
> 
> If that's the case then it's it looks pretty limiting: the
> only thing you can use for the key are strings. I'm not
> sure there's much benefit in changing the behaviour when
> $map.put($this,$that) is available. But it is not hard
> to implement, and if designers are using them then
> consistent use of #set might be desirable.
> 

The limiting is not an impact, since we are talking of
indentifiers. Something like #set $map.$objectKey... is out
of scope and should not be supported.

> > * Patch for ASTReference to use the introspection engine
> >   to allow setValue to a method with a superclass/primitive
> >   signature, e.g.: #set $list.Size = 10
> 
> I agree that primitive properties should be supported,
> but I don't agree with an object with a size(int i)
> method being supported by #set. The #set is for setting
> values of references (a property is a reference) not
> executing arbitrary methods. Why not simply use
> $list.size(1) instead of #set $list.size = 1 ?

OK, I could agree here. We can restrict set and identifiers to 
the bean pattern AND maps. Methods should do the rest...

> 
> > * Patch ASTIdentifier to allow accessing method names not
> >   following bean conventions (e.g. methods not prefixed with
> >   get). This is already possible by just adding empty ()
> >   after the identifier (this makes the node into a ASTMethod!)
> >   - but this is not orthogonal usage: no brakets required
> >   for getXXX methods, but yes for XXX() methods.
> 
> getXXX() are properties and XXX() are not. This was the
> original reason, to follow bean behaviour. Many people
> seem to want this so it could be added.

I guess people are spoiled by the WM form (and are having a hard
time porting from it - as myself). Vel does allow the abbreviated
form of $ref.Xxx to mean $ref.getXxx() and this misleads to 
believe more power behind the introspection...

If you +1 this, I'll provide the patch for the "Given $object.size 
we could search for $object.getSize(), $object.size(), 
$object.get("size")" form.

> 
> > * Alternatively fix ASTIdentifier/MapExecutor not to throw
> >   a ClassCastException.
> > * Set and get object public fields.
> 
> > * Add a feature to ASTReference to access methods/fields of
> >   static classes.
> 
> Static methods work. I use them in Texen/Torque all
> the time. I don't really like being able to access
> public fields.

I meant something like #set $foo = $java.util.Arrays.sort($list)

I can agree to drop the need of accessing public fields in the
Vel environment... If it where to access things like 
javax.swing.SwingConstants.CENTER it would be very desireable...

> 
> jvz.

I'll start as soon as at least you give me a +1 and I have JTest
running again.

:) Christoph

Re: [PROPOSAL] Allignment of introspection

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Christoph Reck wrote:
> 
> This was the only vote, yet recived... Any other votes for the
> other points (in my two initial mails in this thread).
> 
> "Geir Magnusson Jr." wrote:
> >
> > Christoph Reck wrote:
> > >
> > > "Geir Magnusson Jr." wrote:
> > > >
> > > > Here are some ideas to incorporate into this discussion :
> > > >
> > > > I think (and have discussed this with jason, although I won't make any
> > > > claims about his agreement on this) that we might want to not be so
> > > > accomodating to users on the introspection front.  Here's what I am
> > > > thinking about, in no particular order (and some we already do,
> > > > probably) :
> > > >
> > > > 0) I think we should, in our introspection, try the literal
> > > > interpretation of the reference before we wander of in introspection
> > > > flail-land.  This is for speed, and we may already do this.
> > > >
> > >
> > > This would require an additional try/catch. How expensive is a
> > > try/catch compared to:
> > >   two method calls
> > >   a containsKey plus hash lookup
> > >   stringifying the signature
> > >   and doing a get
> > > which is what the introspector currently does. I guess this must
> > > be benchmarked to really find out. Performance issues are future
> >
> > Why is this the only possible implementation solution?
> 
> Well I saw from the commits that you're doing some caching of the
> already cached introspection results...
> So its following the modern approach: speed and waste memory. (I'm
> already locked to 128MB, without funds to upgrade, allways swapping...)

I think I'm at 128MB.  Maybe you should ditch the Windows NT and give
unix a whirl... :)

Seriously, that bit of caching got us a 50% performance improvement. 
Granted. we got hit with a 2x slowdown when we (I) moved the caching
stuff out of init(), but it had to be if we wanted to be thread and
context safe.  So it was a wash, performance wise, but we are no
threadsafe and able to switch data items in the context freely and
remerge templates.

And the new introspection caching is necessary : since the access to the
global introspection cache is synchronized on the cache object, all
servlets, for example, would serialize through there.  I am not sure why
that's a good thing, but feel free to fill me in if I'm mistaken.

In Identifier, we were doing the same introspection over and over again,
so it had to be cached.  In Method, we see above on the serialization of
requests.  Now, in our normal day to day testing, we won't see it, but
when we try to scale big, I think we are going to feel it, I think.  And
finally, in Foreach, it was similar to Method : we were introspecting
over and over again. 
 
> >
> > .
> > >
> > > > 1) I think that we should conform to the bean spec.  There is already
> > > > much literature and support for it.
> > >
> > > +1, plus the notion of maps (currently only applied when getting)
> >
> > You mean for set?  -1
> 
> Well this is democracy...

And I am free to disagree, if you mean in #set(), which wasn't clear
from your answer above.
 

> > > > 2) I think that we should clearly define and document what our
> > > > introspection plan is so a user can code templates to that for speed.
> > > > Interested, Christoph?
> > >
> > > +1
> > > I can summarize what has been agreed here in text form, and
> > > send it to jcastura for revision and embedding.
> >
> > excellent
> 
> No agreement (votes) yet, so no text...
> 
> >
> > > >
> > > > 3) I think we should consider limiting our introspection plan to not try
> > > > anything and everything as a convenience for the users.  Why?  Well,
> > > > consider that we aren't looking for gold or oil here - we are trying to
> > > > match a method call or data access in which a programmer and designer
> > > > have already sat down and agreed on the API they will use between them
> > > > through the context.  Of course, it's a nice feature that you can do
> > > > things like $foo.bar and the programmer has the freedom to use a method
> > > > bar(), getBar() or get("bar") in the object $foo, but does that conform
> > > > to the bean spec anyway?
> > >
> > > No, we are not doing Bean manipulation here, we are pulling data
> > > from bean and map objects and transforming it to text.
> > >
> > > Plus, it should be symmetrical: what I can get with $foo.bar should
> > > also allow a #set($foo.bar = "woobie").
> >
> > Not clear to me why that's true, but that's just my opinion.
> 
> OK, Jose Alberto also said there are read-only breans... So shall we
> keep the unsymmetry?

I agree it's an asymmetry, and I hate asymmetry. I dunno why.  But I am
wary here, for some reason.  Maybe because there is a group here that
believe that adding to the context is dangerous (although I don't), and
maybe I think that it's a compromise that from the template, you can add
'simple' things, but not modify objects or something.

> 
> >
> > > >
> > > > 4) My gut reaction is -1 to #set $foo.bar = RHS for some reason, but
> > > > will think about it. I think because this is a loaded gun of sorts,
> > > > depending upon the class.
> > >
> > > See the symmetrical requirement above.
> > > My example is:
> > >   #set($COLORS = $defaults.ColorLUT)
> > >   #set($COLORS.TEXT = "red")
> > > to do local overrides.
> >
> > Why not simply have the users modify the state of the context objects in
> > a way allowed by the implementing programmer :
> >
> > $foo.setBar("yellow")
> >
> > where setBar returns void.  No muss, no fuss.  Then the inputs can be
> > checked, for example.  Otherwise, you can have things like
> >
> > $colors.text = "frankfurter"
> >
> > and if you really wanted a color... :)
> 
> Using any of the two ways makes no difference.

Hm...  it just struck me  - is there a regularized way of doing that? I
mean, do we have to go on another journey of discovery to make $a.b =
"c" work, trying to see if there is a method of the Class that would
accept that?  That to me sounds dangerous.
 
> >
> >
> > > >
> > > > 5) [wacky]  I can't come up with a convincing example, but consider if
> > > > we allowed users, through a vel.propse property, to tell Velocity to
> > > > *only* try the first approach of the introspection plan and then abandon
> > > > the search.  For specialized cases, I wonder if there would be a good
> > > > performance gain.  It's like the user is saying "Hey, I promise to be
> > > > careful about my reference use in the template, if you (Velocity)
> > > > promise to be quick about it...".
> > >
> > > No, won't work because Java isn't Perl or ksh. Method signature is
> > > an issue. Just think about the requirements of the turbine admin
> > > app; I believe it wouldn't work then. The example that shows what
> > > happens if you do restrict to beans (and strings) can be seen with
> > > JavaScript extensions, any method that thakes something else than
> > > Strings is not accessible (note that *many* bean methods take
> > > primitives and therefore already require introspection).
> >
> > Yes, but I mean to contrain the introspection to our 'first try' and not
> > try other things...
> 
> As I said the first try is prone to fail with Java.

You aren't reading what I wrote, or I am not being clear.  I will assume
the latter : what I meant was that we have a mode, default to off,
settable by a property, where we contrain our hunt to the first try. 
It's not 'bound to fail' because the user is making an active decision -
by turning this 'mode' on they are asking that we do reference analyis
as fast as possible for performance reasons, and in return, the user
will make their templates conform to our documented 'first try' spec.

 
> >
> > > I think theme 0) above should cover the performance issue.
> > >
> > > >
> > > > A little more inline.
> > > >[snip]
> > > > >   ASTSetDirective.render() - to fix a bug I have reported:
> > > > >     + Assigning a NULL value to the context will remove the entry
> > > > >       (since Hashmap does not allow it).
> > > >
> > > > MMmmmm.  Are you sure that's a good idea?  Doesn't that mean something
> > > > could be GC'd if the app was depending upon the context to hold
> > > > something? Granted that would happen if the RHS wasn't nul.... hmm.
> > >
> > > I don't believe the GC is an issue here... If you do a
> > > #set($foo.bar = "anything not null"), the previous content would
> > > be GC'ed the anyway.
> >
> > I know, and I am concerned about $foo.bar on the LHS of a #set, for some
> > reason. I am working on figuring out a covincing argument.
> >
> > > The problem can be seen with the following example:
> > >   Getting element 5 of the testsuite list should returns null
> > >   So what's in foo after "\#set \$foo = \$list.get(5)":
> > >   #set $foo = $list.get(5)
> > >   #if ( $foo )
> > >     \$foo should have contained NULL (has "$foo")!
> > >   #else
> > >     $foo is now correctly NULL.
> > >   #end
> > >
> > > The alternative to removing the map entry is to place a constant
> > > object in there and within the AST evaluation replace it for null
> > > for other usage. I see lot's of complication and no significant
> > > improvement with this approach (other than that the key remains
> > > in the map).
> > >
> > > >[snip]
> > > > >     + possibly: if the rootString is not in the context check with
> > > > >       the classloader if the dotted path (ASTIdentifiers) point
> > > > >       to a static method of a class... similar to the xsl:java
> > > > >       taglib. (if voted for)
> > > >
> > > > Isn't that slow?  The issue would be if something starting with $ wasn't
> > > > a reference (such as alt.on.$sale.now ) how much would this slow
> > > > rendering down?
> > >
> > > I agree, drop this possibility.
> > >
> > > Jason, Daniel, Jon, Bob, Gunnar, any other coments/votes on the
> > > original proposal and the patch summary (in this thread, with the
> > > comments above).
> > >
> > > :) Christoph
> >
> > --
> > Geir Magnusson Jr.                               geirm@optonline.com
> 
> So is the general consensus that we will not support a
>  #set( $foo.bar.crow = "wacky")
> if bar does not support a setcrow (oops I believe we mean setCrow here)?
> 
> If this is so, then the convert.WebMacro will not be very useful.
> 
> :) Christoph

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

Re: [PROPOSAL] Allignment of introspection

Posted by Christoph Reck <Ch...@dlr.de>.
This was the only vote, yet recived... Any other votes for the
other points (in my two initial mails in this thread).

"Geir Magnusson Jr." wrote:
> 
> Christoph Reck wrote:
> >
> > "Geir Magnusson Jr." wrote:
> > >
> > > Here are some ideas to incorporate into this discussion :
> > >
> > > I think (and have discussed this with jason, although I won't make any
> > > claims about his agreement on this) that we might want to not be so
> > > accomodating to users on the introspection front.  Here's what I am
> > > thinking about, in no particular order (and some we already do,
> > > probably) :
> > >
> > > 0) I think we should, in our introspection, try the literal
> > > interpretation of the reference before we wander of in introspection
> > > flail-land.  This is for speed, and we may already do this.
> > >
> >
> > This would require an additional try/catch. How expensive is a
> > try/catch compared to:
> >   two method calls
> >   a containsKey plus hash lookup
> >   stringifying the signature
> >   and doing a get
> > which is what the introspector currently does. I guess this must
> > be benchmarked to really find out. Performance issues are future
> 
> Why is this the only possible implementation solution?

Well I saw from the commits that you're doing some caching of the 
already cached introspection results...
So its following the modern approach: speed and waste memory. (I'm
already locked to 128MB, without funds to upgrade, allways swapping...)

> 
> .
> >
> > > 1) I think that we should conform to the bean spec.  There is already
> > > much literature and support for it.
> >
> > +1, plus the notion of maps (currently only applied when getting)
> 
> You mean for set?  -1

Well this is democracy...

> 
> > >
> > > 2) I think that we should clearly define and document what our
> > > introspection plan is so a user can code templates to that for speed.
> > > Interested, Christoph?
> >
> > +1
> > I can summarize what has been agreed here in text form, and
> > send it to jcastura for revision and embedding.
> 
> excellent

No agreement (votes) yet, so no text...

> 
> > >
> > > 3) I think we should consider limiting our introspection plan to not try
> > > anything and everything as a convenience for the users.  Why?  Well,
> > > consider that we aren't looking for gold or oil here - we are trying to
> > > match a method call or data access in which a programmer and designer
> > > have already sat down and agreed on the API they will use between them
> > > through the context.  Of course, it's a nice feature that you can do
> > > things like $foo.bar and the programmer has the freedom to use a method
> > > bar(), getBar() or get("bar") in the object $foo, but does that conform
> > > to the bean spec anyway?
> >
> > No, we are not doing Bean manipulation here, we are pulling data
> > from bean and map objects and transforming it to text.
> >
> > Plus, it should be symmetrical: what I can get with $foo.bar should
> > also allow a #set($foo.bar = "woobie").
> 
> Not clear to me why that's true, but that's just my opinion.

OK, Jose Alberto also said there are read-only breans... So shall we
keep the unsymmetry?

> 
> > >
> > > 4) My gut reaction is -1 to #set $foo.bar = RHS for some reason, but
> > > will think about it. I think because this is a loaded gun of sorts,
> > > depending upon the class.
> >
> > See the symmetrical requirement above.
> > My example is:
> >   #set($COLORS = $defaults.ColorLUT)
> >   #set($COLORS.TEXT = "red")
> > to do local overrides.
> 
> Why not simply have the users modify the state of the context objects in
> a way allowed by the implementing programmer :
> 
> $foo.setBar("yellow")
> 
> where setBar returns void.  No muss, no fuss.  Then the inputs can be
> checked, for example.  Otherwise, you can have things like
> 
> $colors.text = "frankfurter"
> 
> and if you really wanted a color... :)

Using any of the two ways makes no difference.

> 
> 
> > >
> > > 5) [wacky]  I can't come up with a convincing example, but consider if
> > > we allowed users, through a vel.propse property, to tell Velocity to
> > > *only* try the first approach of the introspection plan and then abandon
> > > the search.  For specialized cases, I wonder if there would be a good
> > > performance gain.  It's like the user is saying "Hey, I promise to be
> > > careful about my reference use in the template, if you (Velocity)
> > > promise to be quick about it...".
> >
> > No, won't work because Java isn't Perl or ksh. Method signature is
> > an issue. Just think about the requirements of the turbine admin
> > app; I believe it wouldn't work then. The example that shows what
> > happens if you do restrict to beans (and strings) can be seen with
> > JavaScript extensions, any method that thakes something else than
> > Strings is not accessible (note that *many* bean methods take
> > primitives and therefore already require introspection).
> 
> Yes, but I mean to contrain the introspection to our 'first try' and not
> try other things...

As I said the first try is prone to fail with Java.

> 
> > I think theme 0) above should cover the performance issue.
> >
> > >
> > > A little more inline.
> > >[snip]
> > > >   ASTSetDirective.render() - to fix a bug I have reported:
> > > >     + Assigning a NULL value to the context will remove the entry
> > > >       (since Hashmap does not allow it).
> > >
> > > MMmmmm.  Are you sure that's a good idea?  Doesn't that mean something
> > > could be GC'd if the app was depending upon the context to hold
> > > something? Granted that would happen if the RHS wasn't nul.... hmm.
> >
> > I don't believe the GC is an issue here... If you do a
> > #set($foo.bar = "anything not null"), the previous content would
> > be GC'ed the anyway.
> 
> I know, and I am concerned about $foo.bar on the LHS of a #set, for some
> reason. I am working on figuring out a covincing argument.
> 
> > The problem can be seen with the following example:
> >   Getting element 5 of the testsuite list should returns null
> >   So what's in foo after "\#set \$foo = \$list.get(5)":
> >   #set $foo = $list.get(5)
> >   #if ( $foo )
> >     \$foo should have contained NULL (has "$foo")!
> >   #else
> >     $foo is now correctly NULL.
> >   #end
> >
> > The alternative to removing the map entry is to place a constant
> > object in there and within the AST evaluation replace it for null
> > for other usage. I see lot's of complication and no significant
> > improvement with this approach (other than that the key remains
> > in the map).
> >
> > >[snip]
> > > >     + possibly: if the rootString is not in the context check with
> > > >       the classloader if the dotted path (ASTIdentifiers) point
> > > >       to a static method of a class... similar to the xsl:java
> > > >       taglib. (if voted for)
> > >
> > > Isn't that slow?  The issue would be if something starting with $ wasn't
> > > a reference (such as alt.on.$sale.now ) how much would this slow
> > > rendering down?
> >
> > I agree, drop this possibility.
> >
> > Jason, Daniel, Jon, Bob, Gunnar, any other coments/votes on the
> > original proposal and the patch summary (in this thread, with the
> > comments above).
> >
> > :) Christoph
> 
> --
> Geir Magnusson Jr.                               geirm@optonline.com

So is the general consensus that we will not support a
 #set( $foo.bar.crow = "wacky")
if bar does not support a setcrow (oops I believe we mean setCrow here)?

If this is so, then the convert.WebMacro will not be very useful.

:) Christoph

Re: [PROPOSAL] Allignment of introspection

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
Christoph Reck wrote:
> 
> "Geir Magnusson Jr." wrote:
> >
> > Here are some ideas to incorporate into this discussion :
> >
> > I think (and have discussed this with jason, although I won't make any
> > claims about his agreement on this) that we might want to not be so
> > accomodating to users on the introspection front.  Here's what I am
> > thinking about, in no particular order (and some we already do,
> > probably) :
> >
> > 0) I think we should, in our introspection, try the literal
> > interpretation of the reference before we wander of in introspection
> > flail-land.  This is for speed, and we may already do this.
> >
> 
> This would require an additional try/catch. How expensive is a
> try/catch compared to:
>   two method calls
>   a containsKey plus hash lookup
>   stringifying the signature
>   and doing a get
> which is what the introspector currently does. I guess this must
> be benchmarked to really find out. Performance issues are future

Why is this the only possible implementation solution?

.
> 
> > 1) I think that we should conform to the bean spec.  There is already
> > much literature and support for it.
> 
> +1, plus the notion of maps (currently only applied when getting)

You mean for set?  -1
 
> >
> > 2) I think that we should clearly define and document what our
> > introspection plan is so a user can code templates to that for speed.
> > Interested, Christoph?
> 
> +1
> I can summarize what has been agreed here in text form, and
> send it to jcastura for revision and embedding.

excellent
 
> >
> > 3) I think we should consider limiting our introspection plan to not try
> > anything and everything as a convenience for the users.  Why?  Well,
> > consider that we aren't looking for gold or oil here - we are trying to
> > match a method call or data access in which a programmer and designer
> > have already sat down and agreed on the API they will use between them
> > through the context.  Of course, it's a nice feature that you can do
> > things like $foo.bar and the programmer has the freedom to use a method
> > bar(), getBar() or get("bar") in the object $foo, but does that conform
> > to the bean spec anyway?
> 
> No, we are not doing Bean manipulation here, we are pulling data
> from bean and map objects and transforming it to text.
> 
> Plus, it should be symmetrical: what I can get with $foo.bar should
> also allow a #set($foo.bar = "woobie").

Not clear to me why that's true, but that's just my opinion.
 
> >
> > 4) My gut reaction is -1 to #set $foo.bar = RHS for some reason, but
> > will think about it. I think because this is a loaded gun of sorts,
> > depending upon the class.
> 
> See the symmetrical requirement above.
> My example is:
>   #set($COLORS = $defaults.ColorLUT)
>   #set($COLORS.TEXT = "red")
> to do local overrides.

Why not simply have the users modify the state of the context objects in
a way allowed by the implementing programmer : 

$foo.setBar("yellow")

where setBar returns void.  No muss, no fuss.  Then the inputs can be
checked, for example.  Otherwise, you can have things like

$colors.text = "frankfurter"

and if you really wanted a color... :)

 
> >
> > 5) [wacky]  I can't come up with a convincing example, but consider if
> > we allowed users, through a vel.propse property, to tell Velocity to
> > *only* try the first approach of the introspection plan and then abandon
> > the search.  For specialized cases, I wonder if there would be a good
> > performance gain.  It's like the user is saying "Hey, I promise to be
> > careful about my reference use in the template, if you (Velocity)
> > promise to be quick about it...".
> 
> No, won't work because Java isn't Perl or ksh. Method signature is
> an issue. Just think about the requirements of the turbine admin
> app; I believe it wouldn't work then. The example that shows what
> happens if you do restrict to beans (and strings) can be seen with
> JavaScript extensions, any method that thakes something else than
> Strings is not accessible (note that *many* bean methods take
> primitives and therefore already require introspection).

Yes, but I mean to contrain the introspection to our 'first try' and not
try other things...
 
> I think theme 0) above should cover the performance issue.
> 
> >
> > A little more inline.
> >[snip]
> > >   ASTSetDirective.render() - to fix a bug I have reported:
> > >     + Assigning a NULL value to the context will remove the entry
> > >       (since Hashmap does not allow it).
> >
> > MMmmmm.  Are you sure that's a good idea?  Doesn't that mean something
> > could be GC'd if the app was depending upon the context to hold
> > something? Granted that would happen if the RHS wasn't nul.... hmm.
> 
> I don't believe the GC is an issue here... If you do a
> #set($foo.bar = "anything not null"), the previous content would
> be GC'ed the anyway.

I know, and I am concerned about $foo.bar on the LHS of a #set, for some
reason. I am working on figuring out a covincing argument.
 
> The problem can be seen with the following example:
>   Getting element 5 of the testsuite list should returns null
>   So what's in foo after "\#set \$foo = \$list.get(5)":
>   #set $foo = $list.get(5)
>   #if ( $foo )
>     \$foo should have contained NULL (has "$foo")!
>   #else
>     $foo is now correctly NULL.
>   #end
> 
> The alternative to removing the map entry is to place a constant
> object in there and within the AST evaluation replace it for null
> for other usage. I see lot's of complication and no significant
> improvement with this approach (other than that the key remains
> in the map).
> 
> >[snip]
> > >     + possibly: if the rootString is not in the context check with
> > >       the classloader if the dotted path (ASTIdentifiers) point
> > >       to a static method of a class... similar to the xsl:java
> > >       taglib. (if voted for)
> >
> > Isn't that slow?  The issue would be if something starting with $ wasn't
> > a reference (such as alt.on.$sale.now ) how much would this slow
> > rendering down?
> 
> I agree, drop this possibility.
> 
> Jason, Daniel, Jon, Bob, Gunnar, any other coments/votes on the
> original proposal and the patch summary (in this thread, with the
> comments above).
> 
> :) Christoph

-- 
Geir Magnusson Jr.                               geirm@optonline.com
Velocity : it's not just a good idea. It should be the law.
http://jakarta.apache.org/velocity

Re: [PROPOSAL] Allignment of introspection

Posted by Christoph Reck <Ch...@dlr.de>.
"Geir Magnusson Jr." wrote:
> 
> Here are some ideas to incorporate into this discussion :
> 
> I think (and have discussed this with jason, although I won't make any
> claims about his agreement on this) that we might want to not be so
> accomodating to users on the introspection front.  Here's what I am
> thinking about, in no particular order (and some we already do,
> probably) :
> 
> 0) I think we should, in our introspection, try the literal
> interpretation of the reference before we wander of in introspection
> flail-land.  This is for speed, and we may already do this.
> 

This would require an additional try/catch. How expensive is a 
try/catch compared to:
  two method calls
  a containsKey plus hash lookup
  stringifying the signature
  and doing a get
which is what the introspector currently does. I guess this must
be benchmarked to really find out. Performance issues are future...

> 1) I think that we should conform to the bean spec.  There is already
> much literature and support for it.

+1, plus the notion of maps (currently only applied when getting)

> 
> 2) I think that we should clearly define and document what our
> introspection plan is so a user can code templates to that for speed.
> Interested, Christoph?

+1
I can summarize what has been agreed here in text form, and 
send it to jcastura for revision and embedding.

> 
> 3) I think we should consider limiting our introspection plan to not try
> anything and everything as a convenience for the users.  Why?  Well,
> consider that we aren't looking for gold or oil here - we are trying to
> match a method call or data access in which a programmer and designer
> have already sat down and agreed on the API they will use between them
> through the context.  Of course, it's a nice feature that you can do
> things like $foo.bar and the programmer has the freedom to use a method
> bar(), getBar() or get("bar") in the object $foo, but does that conform
> to the bean spec anyway?

No, we are not doing Bean manipulation here, we are pulling data 
from bean and map objects and transforming it to text.

Plus, it should be symmetrical: what I can get with $foo.bar should 
also allow a #set($foo.bar = "woobie").

> 
> 4) My gut reaction is -1 to #set $foo.bar = RHS for some reason, but
> will think about it. I think because this is a loaded gun of sorts,
> depending upon the class.

See the symmetrical requirement above.
My example is:
  #set($COLORS = $defaults.ColorLUT)
  #set($COLORS.TEXT = "red")
to do local overrides.

> 
> 5) [wacky]  I can't come up with a convincing example, but consider if
> we allowed users, through a vel.propse property, to tell Velocity to
> *only* try the first approach of the introspection plan and then abandon
> the search.  For specialized cases, I wonder if there would be a good
> performance gain.  It's like the user is saying "Hey, I promise to be
> careful about my reference use in the template, if you (Velocity)
> promise to be quick about it...".

No, won't work because Java isn't Perl or ksh. Method signature is
an issue. Just think about the requirements of the turbine admin
app; I believe it wouldn't work then. The example that shows what 
happens if you do restrict to beans (and strings) can be seen with
JavaScript extensions, any method that thakes something else than
Strings is not accessible (note that *many* bean methods take 
primitives and therefore already require introspection).

I think theme 0) above should cover the performance issue.

> 
> A little more inline.
>[snip]
> >   ASTSetDirective.render() - to fix a bug I have reported:
> >     + Assigning a NULL value to the context will remove the entry
> >       (since Hashmap does not allow it).
> 
> MMmmmm.  Are you sure that's a good idea?  Doesn't that mean something
> could be GC'd if the app was depending upon the context to hold
> something? Granted that would happen if the RHS wasn't nul.... hmm.

I don't believe the GC is an issue here... If you do a
#set($foo.bar = "anything not null"), the previous content would
be GC'ed the anyway.

The problem can be seen with the following example:
  Getting element 5 of the testsuite list should returns null
  So what's in foo after "\#set \$foo = \$list.get(5)":
  #set $foo = $list.get(5)
  #if ( $foo )
    \$foo should have contained NULL (has "$foo")!
  #else
    $foo is now correctly NULL.
  #end

The alternative to removing the map entry is to place a constant
object in there and within the AST evaluation replace it for null
for other usage. I see lot's of complication and no significant 
improvement with this approach (other than that the key remains
in the map).

>[snip]
> >     + possibly: if the rootString is not in the context check with
> >       the classloader if the dotted path (ASTIdentifiers) point
> >       to a static method of a class... similar to the xsl:java
> >       taglib. (if voted for)
> 
> Isn't that slow?  The issue would be if something starting with $ wasn't
> a reference (such as alt.on.$sale.now ) how much would this slow
> rendering down?

I agree, drop this possibility.


Jason, Daniel, Jon, Bob, Gunnar, any other coments/votes on the 
original proposal and the patch summary (in this thread, with the 
comments above).

:) Christoph

Re: [PROPOSAL] Allignment of introspection

Posted by jv...@periapt.com.

On Thu, 7 Dec 2000, Christoph Reck wrote:

> Currently the ASTReference and ASTIdentifier (also ASTSetDirective)
> handle set and get method hardwired and differently. 

They are different for reason. When setting the value of
reference strict bean behaviour is used, there has to
be a setXXX(). For properties, handled by ASTIdentifier
we again use bean behaviour with the extension for Maps.

> ASTIdentifier knows the Map collection class get() method, 
> whereas ASTSetDirective via ASTReference.setValue(...) does not 
> know how to handle the set method of the Map interface.

There is no set method in the Map interface.
 
> An access to $list.size without () throws a ClassCastException
> whereas "#set $list.Size = 10" works (because there is a 
> setSize method).

Right, we are trying to follow standard bean properties.
A List is not a bean, and List.size() is not a property.
But we could certainly extend the search for your example
above. Given $object.size we could search for $object.getSize(),
$object.size(), $object.get("size")

Is that what you're talking about?

> Proposal:
> Allign the handling of references and indentifiers to accept
> getting and setting of:
> * Patch on ASTReference.setValue(...) to allow a set for 
>   objects implementing the Map interface.

So you want 

#set $map.key = $value

instead of

$map.put("key", $value)

If that's the case then it's it looks pretty limiting: the
only thing you can use for the key are strings. I'm not
sure there's much benefit in changing the behaviour when
$map.put($this,$that) is available. But it is not hard
to implement, and if designers are using them then
consistent use of #set might be desirable.

> * Patch for ASTReference to use the introspection engine
>   to allow setValue to a method with a superclass/primitive
>   signature, e.g.: #set $list.Size = 10

I agree that primitive properties should be supported,
but I don't agree with an object with a size(int i)
method being supported by #set. The #set is for setting
values of references (a property is a reference) not
executing arbitrary methods. Why not simply use
$list.size(1) instead of #set $list.size = 1 ?

> * Patch ASTIdentifier to allow accessing method names not 
>   following bean conventions (e.g. methods not prefixed with 
>   get). This is already possible by just adding empty ()
>   after the identifier (this makes the node into a ASTMethod!)
>   - but this is not orthogonal usage: no brakets required
>   for getXXX methods, but yes for XXX() methods.

getXXX() are properties and XXX() are not. This was the
original reason, to follow bean behaviour. Many people
seem to want this so it could be added.

> * Alternatively fix ASTIdentifier/MapExecutor not to throw 
>   a ClassCastException.
> * Set and get object public fields.

> * Add a feature to ASTReference to access methods/fields of 
>   static classes.

Static methods work. I use them in Texen/Torque all
the time. I don't really like being able to access
public fields.

jvz.