"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

Reply via email to