You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by Gabriel Sidler <si...@teamup.ch> on 2002/03/12 19:02:41 UTC

[VelTools] Proposal to change ContextTool

In the context of the new VelocityViewServlet (jakarta-velocity-tool)
we have defined interface ContextTool to be implemented by context
tools (optionally). Context tools that implement this interface can be
handled more efficiently by a tool manager.

I'd like to propose the addition of two new methods to ContextTool
that will further improve the handling of context tools.

    public static void setLogger(ContextToolLogger logger);
    public static boolean isStatic()

See description below for details. setLogger() allows a tool
manager to pass a logger instance to a class of context tools.
isStatic() allows a tool manager to optimize the handling of
a tool class if its methods and fields are all static.

Your feedback is welcome. If nobody disagrees I will go ahead and
implement.

Gabe


<ContextTool.java>
------------------------------------------------------------------------
package org.apache.velocity.tools.view.tools;

import org.apache.velocity.tools.view.context.ViewContext;

/**
  * ContextTool defines a set of methods that allows a context tool
  * manager to efficiently handle context tools.
  */
public interface ContextTool
{
     /**
      * Returns an instance of the context tool. Some tools may simply return
      * themselves from this method others may instantiate new objects
      * to hold the per-request state.
      */
    public Object init(ViewContext context);

    /**
     * Allows a tool manager to pass a logger to the context tool.
     * If a logger is set, it will be used for all instances of
     * this class. If no logger is set, no logging will occur.
     *
     * @param logger A logger.
     */
    public static void setLogger(ContextToolLogger logger);

    /**
     * Allows a tool manager to check if a tool is all static or
     * not. If a tool is not all static, a new instance *must* be
     * created for every template that is processed. If a tool is
     * all static the tool manager *may* optimize the tool handling
     * by working with a single instance for the entire runtime.
     *
     */
    public static boolean isStatic()
}
------------------------------------------------------------------------


<ContextToolLogger.java>
------------------------------------------------------------------------
package org.apache.velocity.tools.view.tools;

import org.apache.velocity.tools.view.context.ViewContext;

/**
  * ContextToolLogger defines an interface for loggers that
  * are passed to context tools.
  */
public interface ContextToolLogger
{

    // Log levels
    public static int INFO;
    public static int WARN;
    public static int ERROR;

    /**
     * Writes a message to the log with the given log level. It is
     * up to the actual implementation of this interface to decide
     * what to do with the log level. It may simply ignore it or it
     * may map it to a log level of the underlying log system if it
     * support the concept of log levels. The supported
     * log levels are {@link #INFO}, {@link #WARN} and
     * {@link #ERROR}.
     */
    public void log(int level, String msg);

}

------------------------------------------------------------------------


--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [VelTools] Proposal to change ContextTool

Posted by Gabriel Sidler <si...@teamup.ch>.
Nathan Bubna wrote:

...

>>     /**
>>      * Returns an instance of the context tool. Some tools may simply return
>>      * themselves from this method others may instantiate new objects
>>      * to hold the per-request state.
>>      */
>>    public Object init(ViewContext context);
> -1
> you know, i've never really been comfortable with this.  To me "init"
> implies that we are using the method to configure the object the method is
> called on, but at present, this method is being used as a factory method to
> create new instances of the implementing class.   That is to me a misnomer.
> I would prefer calling such a method "getInstance" or something similar.


That's a good point! getInstance() is much more intuitive.

>>    /**
>>     * Allows a tool manager to check if a tool is all static or
>>     * not. If a tool is not all static, a new instance *must* be
>>     * created for every template that is processed. If a tool is
>>     * all static the tool manager *may* optimize the tool handling
>>     * by working with a single instance for the entire runtime.
>>     *
>>     */
>>    public static boolean isStatic()
>>
> 
> -1
> ick!  i don't see the point of this.  if a tool is completely static, it
> shouldn't be implementing the ContextTool interface.  my understanding is
> that ContextTool is for tools that are request/session/application
> sensitive.  if a tool is static, it has no use for such stateful objects and
> therefore it is a waste of time to implement ContextTool or anything else.
> the view servlet should just use the same instance of static tools for all
> requests.


You are basically saying that the default behavior of a tool manager should
be to treat a tool as static unless it implements some interface that tells
it otherwise. That's a possible approach and seems reasonable for all tools
I can think of a the moment.

The other aspect of a tool is if it can log or not. Maybe we should treat
this independently of the static/non-static aspect.

Gabe



> 
> [snip]
> 
> Nathan Bubna
> nathan@esha.com
> 
> 
> 
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
> 
> 


-- 
--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [VelTools] Proposal to change ContextTool

Posted by Nathan Bubna <na...@esha.com>.
Gabe said:
[snip]
> > at present, the tools we've talked about are all request oriented or
global
> > (aka static).  in the velstruts app i've developed, one of the tools i
wrote
> > is only session sensitive.
>
>
> What does this tool? I am curious because I couldn't think of any such
> tool.

well, the simplest example would be a tool that accesses user information
loaded into the session (we always have tools like this our Turbine/Velocity
apps).  such tools needed only be initialized when the user logins in
(starts a new session), but the only session tool i currently have in a
Struts/Velocity app is a FoodlistTool.  it's only responsibility is
accessing and analyzing a list of Food objects that is stored in the
session.  once the tool has been initialized and grabbed a reference to the
foodlist from the session, it doesn't need reinitialized.

> > as for the scope being 'fixed by the design', i don't think that must
> > necessarily be true.  if my proposed behavior is accepted, i would
happily
> > make DateTool implement ContextTool so that instances returned by
> > 'getInstance' would look for a Locale in the request or session to use
> > instead of the default.  that way, those developers who wish to use
DateTool
> > as global (statically) or request scope may simply specify the desired
scope
> > in the config file.
>
>
> I see your point in theorie. I am just not convinced yet that it
> addresses are real need. I cannot imaging a real-world tool where
> the scope it not fixed by the design of the tool.

DateTool  isn't real-world?  hmm.  perhaps if you describe what you mean by
a 'real-world tool' then i'll be able to think of an example.

granted, i think tools which can reasonably vary in scope will be the
exception rather than the rule, and you can certainly solve the DateTool
situation i describe by having people who want a request scope DateTool
extend the current version to implement the appropriate interface.  However,
i would prefer to avoid putting people in situations like that, and i don't
see the harm in allowing them to specify scope in toolbox.xml.  there's just
something intuitive to me about that (perhaps due to time spent using
Turbine).  besides, i like being able to look at the config file and see
what scope each tool is without have to look at each file.

i guess the bottom line(s) IMO is:
-the present treatment of tools is inadequate.
-we would be mistreating and ignoring a significant class of tools by not
supporting session scope tools.
-there isn't really any serious need for supporting variable scope tools.
however, i think that it could definitely come in handy sometimes, and i
still don't see the harm in it.  at the very least, it would allow us to
provide a more useful and flexible DateTool.  for custom tools created by
developers, this will probably almost always be a moot point as they'll have
easy control over the source of their tools.

> > ...in any case, i would
> > suggest that we make the default scope (the setting if no scope is
specified
> > in the config) be request.
>
> Well, in one of your last mails you convinced my of the opposite ;-)
> Assuming request scope might seem to be safe default behavior, BUT
> if a tool doesn't implement any interfaces it won't be able to get
> an parameters passed from the tool manager. If it doesn't get any
> parameters, it doesn't interact with the environment. Isn't it pretty
> safe to assume that in such a situation the tool is global?

yeah, it is.  i don't know what i was thinking.

> With my current understanding I would vote for global scope as default.

you're quite right.

> Ok, I'll wait for your code and then will go from there.

soon now...

Nathan Bubna
nathan@esha.com


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [VelTools] Proposal to change ContextTool

Posted by Gabriel Sidler <si...@teamup.ch>.
Nathan Bubna wrote:

> Gabe said:
> 
>>Isn't the scope of a tool given by its design? I mean, if a tool requires
>>access to request data its scope is obviously 'request', etc. So, why
>>
> allow
> 
>>the user to configure something that is already fixed by the design of the
>>tool? Maybe my imagination is too limited?
>>
> 
> hmm.  i would say that the scope of a tool most certainly can and should be
> closely tied to its design, but i can envision situations where a tool can
> be designed to work in multiple scopes.  in these situations (and i think
> generally as well), being able to set the scope of a tool in the config file
> can be both useful and more efficient.


I am little concerned that we make configuration more compliated than
necessary, but if there there is a real benefit for it...


> at present, the tools we've talked about are all request oriented or global
> (aka static).  in the velstruts app i've developed, one of the tools i wrote
> is only session sensitive. 


What does this tool? I am curious because I couldn't think of any such
tool.

> it cares neither about requests, nor is it
> static.  for this tool, i need to implement ContextTool (or something like
> it in order to get access to the session, but it is a waste of time to get a
> new instance for each request.   i would like to be able to configure the
> toolbox so that a new tool is created only per session.  my proposal would
> handle this efficiently, without having to check the 'design' of every tool
> on every request.


Well, checking of the 'design' should happen once at loadtime.


> as for the scope being 'fixed by the design', i don't think that must
> necessarily be true.  if my proposed behavior is accepted, i would happily
> make DateTool implement ContextTool so that instances returned by
> 'getInstance' would look for a Locale in the request or session to use
> instead of the default.  that way, those developers who wish to use DateTool
> as global (statically) or request scope may simply specify the desired scope
> in the config file.


I see your point in theorie. I am just not convinced yet that it
addresses are real need. I cannot imaging a real-world tool where
the scope it not fixed by the design of the tool.

> 
> 
>>By implementing specific interfaces we can give the tool manager hints
>>
> about
> 
>>how to handle the tool. If a tool doesn't implement any interface, some
>>default behaviour would be applied.
>>
> 
> yes, but even if we have a different interface for every scope (or at least
> the non-static scopes), it is inefficient to check every tool for the
> appropriate hints on every request.  it is more efficient and gives greater
> flexibility if we specify scope aforehand.  at the very least i would want
> the toolbox manager to check those 'hints' upon initial loading of the
> toolbox and maintain the three lists of tools as i proposed.


Of course, there shouldn't be any checking at every request.


>>You mention Turbine's PullService: Can you show some cases where it makes
>>sense to let the user choose the scope of a tool?
>>
> 
> user?!  i would hardly call editing the toolbox config the job of a user!  i
> would expect a developer would typically be responsible for that.  i'd even
> be surprised to see a designer messing with that.  in any case, i would
> suggest that we make the default scope (the setting if no scope is specified
> in the config) be request. 


Well, in one of your last mails you convinced my of the opposite ;-)
Assuming request scope might seem to be safe default behavior, BUT
if a tool doesn't implement any interfaces it won't be able to get
an parameters passed from the tool manager. If it doesn't get any
parameters, it doesn't interact with the environment. Isn't it pretty
safe to assume that in such a situation the tool is global?
With my current understanding I would vote for global scope as default.


> that is essentially what the current behaviour
> is, and that way the worst that happens is that getToolboxContent is more
> inefficient than need be.
> 
> i'm writing the code for this right now so you can see it.  i'll post as
> soon as its ready.



Ok, I'll wait for your code and then will go from there.

Gabe




--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [VelTools] Proposal to change ContextTool

Posted by Nathan Bubna <na...@esha.com>.
Gabe said:
> Isn't the scope of a tool given by its design? I mean, if a tool requires
> access to request data its scope is obviously 'request', etc. So, why
allow
> the user to configure something that is already fixed by the design of the
> tool? Maybe my imagination is too limited?

hmm.  i would say that the scope of a tool most certainly can and should be
closely tied to its design, but i can envision situations where a tool can
be designed to work in multiple scopes.  in these situations (and i think
generally as well), being able to set the scope of a tool in the config file
can be both useful and more efficient.

at present, the tools we've talked about are all request oriented or global
(aka static).  in the velstruts app i've developed, one of the tools i wrote
is only session sensitive.  it cares neither about requests, nor is it
static.  for this tool, i need to implement ContextTool (or something like
it in order to get access to the session, but it is a waste of time to get a
new instance for each request.   i would like to be able to configure the
toolbox so that a new tool is created only per session.  my proposal would
handle this efficiently, without having to check the 'design' of every tool
on every request.

as for the scope being 'fixed by the design', i don't think that must
necessarily be true.  if my proposed behavior is accepted, i would happily
make DateTool implement ContextTool so that instances returned by
'getInstance' would look for a Locale in the request or session to use
instead of the default.  that way, those developers who wish to use DateTool
as global (statically) or request scope may simply specify the desired scope
in the config file.

> By implementing specific interfaces we can give the tool manager hints
about
> how to handle the tool. If a tool doesn't implement any interface, some
> default behaviour would be applied.

yes, but even if we have a different interface for every scope (or at least
the non-static scopes), it is inefficient to check every tool for the
appropriate hints on every request.  it is more efficient and gives greater
flexibility if we specify scope aforehand.  at the very least i would want
the toolbox manager to check those 'hints' upon initial loading of the
toolbox and maintain the three lists of tools as i proposed.

> You mention Turbine's PullService: Can you show some cases where it makes
> sense to let the user choose the scope of a tool?

user?!  i would hardly call editing the toolbox config the job of a user!  i
would expect a developer would typically be responsible for that.  i'd even
be surprised to see a designer messing with that.  in any case, i would
suggest that we make the default scope (the setting if no scope is specified
in the config) be request.  that is essentially what the current behaviour
is, and that way the worst that happens is that getToolboxContent is more
inefficient than need be.

i'm writing the code for this right now so you can see it.  i'll post as
soon as its ready.

Nathan Bubna
nathan@esha.com




--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [VelTools] Proposal to change ContextTool

Posted by Jon Scott Stevens <jo...@latchkey.com>.
on 3/12/02 11:05 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:

> You mention Turbine's PullService: Can you show some cases where it makes
> sense to let the user choose the scope of a tool?
> 
> Gabe

It doesn't.

-jon


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [VelTools] Proposal to change ContextTool

Posted by Gabriel Sidler <si...@teamup.ch>.
Nathan Bubna wrote:

[...]

> well, since we're talking about tool configuration, lifecycle, etc., it's
> given me an idea.
> this is off the top of my head, but i guess you can consider this something
> of a counter proposal of sorts to Gabriel's proposed ContextTool changes...
> 
> one of things i appreciate about Turbine's tool management system
> (PullService) is the ability to specify the scope of my tools (session,
> request, global).   i think something similar to that would be relatively
> easy to implement with the ToolboxManager.  all we would need to specify the
> desired scope for our tools in the toolbox.xml as such:
> 
> <toolbox>
>   <tools>
>    <object>
>      <contextkey>link</contextkey>
>      <scope>request</scope>
>      <class>...LinkTool</class>
>    </object>
>    <object>
>      <contextkey>math</contextkey>
>      <scope>global</scope>
>      <class>...MathTool</class>
>    </object>
>   </tools>
> </toolbox>
> 
> in ToolboxManager we'd load three lists of tools, instead of one.  the
> global tools would be initialized immediately, and then on each call of
> getToolboxContext,  we would
> 1. add the initialized global tools to the ToolboxContext
> 2. check the session for a list of stored session tools
>         if the list is not there,
>             initialize any session tools
>             store them in a list in the session
>             add the initialized tools to the ToolboxContext
>         if the list is there,
>             add the already initialized tools in it to the ToolboxContext
> 3. initialize and add all request tools to the ToolboxContext
> 
> hmm, i probably should find a better word than "initialize" in the above
> proposal, but my brain is failing me on that.  so i guess i need to explain
> that tools which wish to be "initialized" need to implement ContextTool.
> they would be "initialized," by calling 'getInstance' or 'init' or whatever
> we call it, and using the "initialized" tool that method returns.
> otherwise, the instance created during the first loading of the toolbox will
> be used.
> actually, if that's the case, then we ought to insist (and enforce) that
> session and request tools must implement ContextTool.
> 
> let me know what you guys think about this, but i'm probably gonna implement
> it myself first thing tomorrow to see if i really do like this idea.   if i
> do like it, i'll be more than happy to share the changes i make.



Isn't the scope of a tool given by its design? I mean, if a tool requires
access to request data its scope is obviously 'request', etc. So, why allow
the user to configure something that is already fixed by the design of the
tool? Maybe my imagination is too limited?

By implementing specific interfaces we can give the tool manager hints about
how to handle the tool. If a tool doesn't implement any interface, some
default behaviour would be applied.

You mention Turbine's PullService: Can you show some cases where it makes
sense to let the user choose the scope of a tool?

Gabe






> 
> Nathan Bubna
> nathan@esha.com
> 
> 
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
> 
> 

--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [VelTools] Proposal to change ContextTool

Posted by Gabriel Sidler <si...@teamup.ch>.
Nathan, Geir,
based on your feedback I realized that my proposal from
yesterday regarding the ContextTool interface has some
deficiencies. There's actually more to it than you think
at first. I attempted to look at the problem a little more
systematically. My findings including a proposal on how
to proceed are below.

I am looking forward to your feedback.

Gabe



Design Goals
============

- Tools should be reusable in different frameworks.
- The handling of tools should be efficient:
    - as few instances as needed
    - efficient assembly of a Velocity context for a particular request
- Enable context tools to log
- A tool manager must be able to instantiate context tools
   and setup the Velocity context automatically.
- Any object with public methods and a constructor without parameters
   should be usable as a context tool.



Aspects of Context Tools
========================
I see the following three relevant aspects of a context tool from
the point of view of a tool manager:

1) Scope
--------
The scope of a context tool:
a) request
b) global [default]
Others? Like session scope?

2) Data Passed
--------------
The type of data passed to a context tool:
a) nothing [default]
b) Servlet request, servlet session, servlet context
Others?

3) Loggable
-----------
To log or not to log.



Implementation
==============
I propose the following implementation approach:

1) By default context tools receive the following treatment by tool
    managers:

    a) The tool is assumed to be of global scope, that is, one instance is
       used for all requests during the entire runtime of the program.
    b) The tool manager instantiates an object using a public constructor
       with no parameters. No data is passed.
    c) No logger is passed.

    The default case applies if none of the interfaces defined below
    are implemented. This allows any object with a public constructor
    without parameters and public methods to be used as a context tool.


2) Every context tool class that needs non-default handling regarding
    scope, data passed, or logging must implement an interface that
    establishes a contract between context tool managers and a class of
    context tools.

    For context tools used in a servlet environment the following concrete
    interface is proposed:

      public interface ServletContextTool
      {
         // Define possible scopes
         public static int REQUEST;
         public static int GLOBAL;

         // Methods
         public static getInstance(ViewContext c);
         public static getScope(int scope);
      }

    Interfaces for other environments can be defined as needed. Such
    an interface essentially establishes a category of context tools.


3) Any context tool class that wishes to have access to a logger, must
    implement the following interface. In environments where logging
    is supported, the tool manager *may* use this interface to pass a
    logger to the context tool. The context tool must be able to deal
    with situations where no logger is passed.

      public interface LogEnabled
      {
          public static void setLogger(Logger logger);
      }

     Logger is an interface that is to be defined.


4) Implementations of tool managers need to declare which categories
    of context tools they support. Any tool category unknown to the tool
    manager receives default treatments as defined under 1)



A few notes about the proposed implementation:

- I believe the above proposal establishes a flexible and extensible
   framework for working with context tools. For the large category of
   static, non-logging tools it does require any special measures.
   For tools with special needs special handling is possible.
   Above proposal is not fundamentally different from what we do today,
   but it outlines it more formally.

- The above approach assumes that the scope of a context tool is
   given by the design of the tool and is not something that needs to
   be configurable by the user. I am looking forward to some feedback
   from Nathan on this because he proposed yesterday to allow the
   user to define scopes. I am wondering what examples of tools would
   be where it makes sense to have the user choose the scope.

- The logging aspect is really orthogonal to the other two aspects.
   Therefore, I defined an interface that deals just with this aspect
   (based on best practices recommended by the Avalon framework).


--
Gabriel Sidler
Software Engineer, Eivycom GmbH, Zurich, Switzerland


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [VelTools] Proposal to change ContextTool

Posted by Nathan Bubna <na...@esha.com>.
Geir said:
[...]
> >>     public static boolean isStatic()
> >
> > -1
> > ick!  i don't see the point of this.  if a tool is completely static, it
> > shouldn't be implementing the ContextTool interface.  my understanding
is
> > that ContextTool is for tools that are request/session/application
> > sensitive.  if a tool is static, it has no use for such stateful objects
and
> > therefore it is a waste of time to implement ContextTool or anything
else.
> > the view servlet should just use the same instance of static tools for
all
> > requests.
>
> You are right - I didn't think about it that way - my thoughts surrounding
> this really had to do with lifecycle - and I think the right name here is
> 'single instance'  (note I avoided the word 'singleton')
>
> When you list your tools in the configuration, you can indicate if the
tool
> is a single instance or not, -> the toolmanager reuses a single instance
in
> each context, or must make a fresh instance for each.
>
> That's the important aspect here, or believe so, because it means if you
> have some heavy tools that do an incredible amount of stuff once, then you
> can denote them as 'single instance' in the config, and not worry that you
> get hammered with each use.

well, since we're talking about tool configuration, lifecycle, etc., it's
given me an idea.
this is off the top of my head, but i guess you can consider this something
of a counter proposal of sorts to Gabriel's proposed ContextTool changes...

one of things i appreciate about Turbine's tool management system
(PullService) is the ability to specify the scope of my tools (session,
request, global).   i think something similar to that would be relatively
easy to implement with the ToolboxManager.  all we would need to specify the
desired scope for our tools in the toolbox.xml as such:

<toolbox>
  <tools>
   <object>
     <contextkey>link</contextkey>
     <scope>request</scope>
     <class>...LinkTool</class>
   </object>
   <object>
     <contextkey>math</contextkey>
     <scope>global</scope>
     <class>...MathTool</class>
   </object>
  </tools>
</toolbox>

in ToolboxManager we'd load three lists of tools, instead of one.  the
global tools would be initialized immediately, and then on each call of
getToolboxContext,  we would
1. add the initialized global tools to the ToolboxContext
2. check the session for a list of stored session tools
        if the list is not there,
            initialize any session tools
            store them in a list in the session
            add the initialized tools to the ToolboxContext
        if the list is there,
            add the already initialized tools in it to the ToolboxContext
3. initialize and add all request tools to the ToolboxContext

hmm, i probably should find a better word than "initialize" in the above
proposal, but my brain is failing me on that.  so i guess i need to explain
that tools which wish to be "initialized" need to implement ContextTool.
they would be "initialized," by calling 'getInstance' or 'init' or whatever
we call it, and using the "initialized" tool that method returns.
otherwise, the instance created during the first loading of the toolbox will
be used.
actually, if that's the case, then we ought to insist (and enforce) that
session and request tools must implement ContextTool.

let me know what you guys think about this, but i'm probably gonna implement
it myself first thing tomorrow to see if i really do like this idea.   if i
do like it, i'll be more than happy to share the changes i make.

Nathan Bubna
nathan@esha.com


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [VelTools] Proposal to change ContextTool

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
On 3/12/02 2:12 PM, "Nathan Bubna" <na...@esha.com> wrote:

> Gabriel Sidler wrote:
> [snip]
>> I'd like to propose the addition of two new methods to ContextTool
>> that will further improve the handling of context tools.
> 
> hmm.  i'm not a fan of complicating interfaces if it can be avoided, and
> i've never been thrilled with the current ContextTool interface either.
> more below...
> 
> [snip]
>> /**
>>   * ContextTool defines a set of methods that allows a context tool
>>   * manager to efficiently handle context tools.
>>   */
>> public interface ContextTool
>> {
>>      /**
>>       * Returns an instance of the context tool. Some tools may simply
> return
>>       * themselves from this method others may instantiate new objects
>>       * to hold the per-request state.
>>       */
>>     public Object init(ViewContext context);
> 
> -1
> you know, i've never really been comfortable with this.  To me "init"
> implies that we are using the method to configure the object the method is
> called on, but at present, this method is being used as a factory method to
> create new instances of the implementing class.   That is to me a misnomer.
> I would prefer calling such a method "getInstance" or something similar.

+1 your -1 - I agree that a name change would be a good thing

> 
>>     /**
>>      * Allows a tool manager to pass a logger to the context tool.
>>      * If a logger is set, it will be used for all instances of
>>      * this class. If no logger is set, no logging will occur.
>>      *
>>      * @param logger A logger.
>>      */
>>     public static void setLogger(ContextToolLogger logger);
> 
> +0
> hmm.  yes, we should provide some hook for tools that output log messages to
> use a common log system, but i'm not convinced that this is the best way.
> however, neither do i have an alternative suggestion at the moment.  i need
> to chew on this one.
> 
>>     /**
>>      * Allows a tool manager to check if a tool is all static or
>>      * not. If a tool is not all static, a new instance *must* be
>>      * created for every template that is processed. If a tool is
>>      * all static the tool manager *may* optimize the tool handling
>>      * by working with a single instance for the entire runtime.
>>      *
>>      */
>>     public static boolean isStatic()
> 
> -1
> ick!  i don't see the point of this.  if a tool is completely static, it
> shouldn't be implementing the ContextTool interface.  my understanding is
> that ContextTool is for tools that are request/session/application
> sensitive.  if a tool is static, it has no use for such stateful objects and
> therefore it is a waste of time to implement ContextTool or anything else.
> the view servlet should just use the same instance of static tools for all
> requests.

You are right - I didn't think about it that way - my thoughts surrounding
this really had to do with lifecycle - and I think the right name here is
'single instance'  (note I avoided the word 'singleton')

When you list your tools in the configuration, you can indicate if the tool
is a single instance or not, -> the toolmanager reuses a single instance in
each context, or must make a fresh instance for each.

That's the important aspect here, or believe so, because it means if you
have some heavy tools that do an incredible amount of stuff once, then you
can denote them as 'single instance' in the config, and not worry that you
get hammered with each use.

Geir

> 
> [snip]
> 
> Nathan Bubna
> nathan@esha.com
> 
> 
> 
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
> 

-- 
Geir Magnusson Jr.                       geirm@optonline.net
System and Software Consulting
You're going to end up getting pissed at your software
anyway, so you might as well not pay for it. Try Open Source.



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [VelTools] Proposal to change ContextTool

Posted by Nathan Bubna <na...@esha.com>.
Gabriel Sidler wrote:
[snip]
> I'd like to propose the addition of two new methods to ContextTool
> that will further improve the handling of context tools.

hmm.  i'm not a fan of complicating interfaces if it can be avoided, and
i've never been thrilled with the current ContextTool interface either.
more below...

[snip]
> /**
>   * ContextTool defines a set of methods that allows a context tool
>   * manager to efficiently handle context tools.
>   */
> public interface ContextTool
> {
>      /**
>       * Returns an instance of the context tool. Some tools may simply
return
>       * themselves from this method others may instantiate new objects
>       * to hold the per-request state.
>       */
>     public Object init(ViewContext context);

-1
you know, i've never really been comfortable with this.  To me "init"
implies that we are using the method to configure the object the method is
called on, but at present, this method is being used as a factory method to
create new instances of the implementing class.   That is to me a misnomer.
I would prefer calling such a method "getInstance" or something similar.

>     /**
>      * Allows a tool manager to pass a logger to the context tool.
>      * If a logger is set, it will be used for all instances of
>      * this class. If no logger is set, no logging will occur.
>      *
>      * @param logger A logger.
>      */
>     public static void setLogger(ContextToolLogger logger);

+0
hmm.  yes, we should provide some hook for tools that output log messages to
use a common log system, but i'm not convinced that this is the best way.
however, neither do i have an alternative suggestion at the moment.  i need
to chew on this one.

>     /**
>      * Allows a tool manager to check if a tool is all static or
>      * not. If a tool is not all static, a new instance *must* be
>      * created for every template that is processed. If a tool is
>      * all static the tool manager *may* optimize the tool handling
>      * by working with a single instance for the entire runtime.
>      *
>      */
>     public static boolean isStatic()

-1
ick!  i don't see the point of this.  if a tool is completely static, it
shouldn't be implementing the ContextTool interface.  my understanding is
that ContextTool is for tools that are request/session/application
sensitive.  if a tool is static, it has no use for such stateful objects and
therefore it is a waste of time to implement ContextTool or anything else.
the view servlet should just use the same instance of static tools for all
requests.

[snip]

Nathan Bubna
nathan@esha.com



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [VelTools] Proposal to change ContextTool

Posted by "Geir Magnusson Jr." <ge...@optonline.net>.
+1


On 3/12/02 1:02 PM, "Gabriel Sidler" <si...@teamup.ch> wrote:

> In the context of the new VelocityViewServlet (jakarta-velocity-tool)
> we have defined interface ContextTool to be implemented by context
> tools (optionally). Context tools that implement this interface can be
> handled more efficiently by a tool manager.
> 
> I'd like to propose the addition of two new methods to ContextTool
> that will further improve the handling of context tools.
> 
>   public static void setLogger(ContextToolLogger logger);
>   public static boolean isStatic()
> 
> See description below for details. setLogger() allows a tool
> manager to pass a logger instance to a class of context tools.
> isStatic() allows a tool manager to optimize the handling of
> a tool class if its methods and fields are all static.
> 
> Your feedback is welcome. If nobody disagrees I will go ahead and
> implement.
> 
> Gabe
> 
> 
> <ContextTool.java>
> ------------------------------------------------------------------------
> package org.apache.velocity.tools.view.tools;
> 
> import org.apache.velocity.tools.view.context.ViewContext;
> 
> /**
> * ContextTool defines a set of methods that allows a context tool
> * manager to efficiently handle context tools.
> */
> public interface ContextTool
> {
>    /**
>     * Returns an instance of the context tool. Some tools may simply return
>     * themselves from this method others may instantiate new objects
>     * to hold the per-request state.
>     */
>   public Object init(ViewContext context);
> 
>   /**
>    * Allows a tool manager to pass a logger to the context tool.
>    * If a logger is set, it will be used for all instances of
>    * this class. If no logger is set, no logging will occur.
>    *
>    * @param logger A logger.
>    */
>   public static void setLogger(ContextToolLogger logger);
> 
>   /**
>    * Allows a tool manager to check if a tool is all static or
>    * not. If a tool is not all static, a new instance *must* be
>    * created for every template that is processed. If a tool is
>    * all static the tool manager *may* optimize the tool handling
>    * by working with a single instance for the entire runtime.
>    *
>    */
>   public static boolean isStatic()
> }
> ------------------------------------------------------------------------
> 
> 
> <ContextToolLogger.java>
> ------------------------------------------------------------------------
> package org.apache.velocity.tools.view.tools;
> 
> import org.apache.velocity.tools.view.context.ViewContext;
> 
> /**
> * ContextToolLogger defines an interface for loggers that
> * are passed to context tools.
> */
> public interface ContextToolLogger
> {
> 
>   // Log levels
>   public static int INFO;
>   public static int WARN;
>   public static int ERROR;
> 
>   /**
>    * Writes a message to the log with the given log level. It is
>    * up to the actual implementation of this interface to decide
>    * what to do with the log level. It may simply ignore it or it
>    * may map it to a log level of the underlying log system if it
>    * support the concept of log levels. The supported
>    * log levels are {@link #INFO}, {@link #WARN} and
>    * {@link #ERROR}.
>    */
>   public void log(int level, String msg);
> 
> }
> 
> ------------------------------------------------------------------------
> 
> 
> --
> Gabriel Sidler
> Software Engineer, Eivycom GmbH, Zurich, Switzerland
> 
> 
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
> 

-- 
Geir Magnusson Jr.                                     geirm@optonline.net
System and Software Consulting
"Now what do we do?"


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>