[
https://issues.apache.org/jira/browse/VELOCITY-704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12705655#action_12705655
]
Nathan Bubna commented on VELOCITY-704:
---------------------------------------
Ay, a speed hit is expected. Though i think it is worth it for the features
and simplification it offers. I do hope we can improve on the speed. Of
course, if you don't use the feature and want faster macros, the simplest fix
is:
macro.provide.scope.control = false
then it's just a boolean condition check in both preRender and postRender.
Also, in 2.0, some of the lost ground should be recovered (i expect) since 1.7
has a lot of now-deprecated functionality that has been removed from the 2.0
branch (velocimacro.context.localscope, velocityCount, velocityHasNext, etc
As for particular speed improvements, i haven't done any testing or much
thinking yet, but when i first wrote this i did consider replacing a lot of
those:
if (obj instanceof Scope) { this } else { that }
with
try { this } catch (ClassCastException) { that }
since i think it will be rather rare (and increasingly so in the future) that
people will override Scope instances in their context. But, without doing any
performance testing to compare the two approaches, i figured it was best to
start out doing it "the right way" and worry about optimizing later. The one
other idea i had was to keep a small map of "scoped directive" stacks (or
something like that) in the InternalContextAdapter and lazily create Scope
objects as requested. It seemed more difficult to do at the time though, and
again, without performance numbers to compare it was hard to justify the
effort. Anyway, all that is to say that i think we can do better here
performance-wise; it'll just take a little testing and refactoring. :)
> VTL Simplicity - "Control" objects
> ----------------------------------
>
> Key: VELOCITY-704
> URL: https://issues.apache.org/jira/browse/VELOCITY-704
> Project: Velocity
> Issue Type: New Feature
> Components: Engine
> Reporter: Nathan Bubna
> Assignee: Nathan Bubna
> Fix For: 1.7, 2.0
>
>
> In the discussion for VELOCITY-680, Claude suggested the addition of what i'm
> calling "control" objects as a solution. These would have the same name as
> the block directive or macro to which they belong. At a minimum, these
> would provide get(key), set(key, value) and stop() methods to control the
> reference scoping and execution of the block to which they belong.
> Directives could extend the basic control object to provide additional
> functions, such as index() and hasNext() for #foreach. Here's some examples:
> #foreach( $user in $users )
> $user#if( $foreach.hasNext ), #end
> #if( $foreach.count > 10 ) $foreach.stop() #end
> #end
> #macro( foo $bar )
> blah blah #if( $bar == 'bar' ) $foo.stop() #end
> #set( $foo.woogie = 'woogie' )
> $foo.woogie
> #end
> #foreach( $item in $list )
> #set( $outer = $foreach )
> #foreach( $attr in $item.attributes )
> #if ( $attr == $null ) $outer.stop()#end
> #end
> #end
> ------foo.vm---------
> blah blah $template.stop() blah
> ------------------------
> #define( $foo )
> blah blah $define.stop() blah
> #end
> This could allow us to greatly simplify all sorts of things. We could remove
> the #break, #stop and #return directives. We would no longer need to have
> "local" contexts for foreach loops or macros; instead users could set and get
> local variables directly in the provided namespace. All else would be
> global. This may even cut down our internal code complexity a fair bit.
> It'll certainly obviate the need for several configuration properties and
> internal contexts. Everything becomes much more explicit, obvious and
> robust. I also don't think it looks ugly. :)
> We would, of course, have to make sure that the StopExceptions thrown by
> stop() aren't wrapped into MethodInvocationExceptions. We'd have to make the
> directives clean up their control when done rendering, and if they're nested
> in a directive of the same type, then they should save and restore the
> reference to the parent control. We'd also have to figure out a good
> default name to give the control objects for the top-level control object,
> and whether it would be different than the name of the control object used
> during a #parse call. $template? $parse? $velocity? If we wanted to use
> $template--which i think works well for both top-level and #parse--then we'd
> probably have to make it configurable, since that's likely to conflict. And
> if we make that configurable, i suppose we may as well make it configurable
> for the others too.
> I'm struggling to think of any real downside to this. Most of the replaced
> features (implicit macro localscope, #stop, #break, $velocityHasNext) are
> either not default behavior or are new features. I'd wager that most people
> would only have to change $velocityCount to $foreach.count. Even that's no
> big deal, since this would be for a major version change. , The worst i can
> think of is the fact that for a couple of these controls it would mean a few
> more keystrokes. Considering all the gains in extensibility, explicitness
> and simplification (for us and users), i think it's worth a few keystrokes.
> What do you guys think?
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
