On 10/28/05, Henning P. Schmiedehausen <[EMAIL PROTECTED]> wrote: > Nathan Bubna <[EMAIL PROTECTED]> writes: > > I admit that I didn't look too close at the javadocs in > Configurable. As it is documented, I'd keep it as it is. > > However, this still leaves a feeling of unease. If this would be a > proper lifecycle for the tool, then the method would be called, no > matter what the contents of the parameters map is. If it is empty, so > be it. Having a method that might or might not be called depending on > the contents of the parameters array (or the parameters in the toolbox > definition) will IMHO lead to "gotchas" at some point.
ok, this may be splitting hairs, but i think there's a significant difference between saying a method "might or might not be called depending on the contents of the parameter map", as opposed to "the method will only be called if parameters exist." i.e. i see the action as being triggered by the presence of parameters, not by their contents. i consider the configuration to be evidence of the user's desire for an action to occur, not the fact that a tool implements Configurable. i know that's debatable with the current state of things, but i'm thinking toward the future. i'd like to ditch the ViewTool and Configurable interfaces for Tools 2.0 (and maybe even for 1.3, with deprecation). > Best regards > Henning > > > > >On 10/27/05, Henning P. Schmiedehausen <[EMAIL PROTECTED]> wrote: > >> [EMAIL PROTECTED] writes: > >> > >> >URL: http://svn.apache.org/viewcvs?rev=321226&view=rev > >> >Log: > >> >make sure a null parameter map is never passed to configurable tools > >> > >> Ugh! This also changes the sematics of the method call! If there are > >> no parameters, then configure is not called at all. What about tools > >> that rely on the method being called no matter what parameters are > >> passed in? > > >i'd say that's a really bad idea given the way this method is > >implemented and very clearly documented. :) the idea is that > >configure() is a hook for *optional* configuration parameters > >specified in the toolbox.xml. a tool that relies on having an empty > >parameter map passed to it is designed wrong. configurability should > >be for changing default settings, not initializing a tool. > > >> I'd change that to > >> > >> if (configurable) > >> { > >> Map params = (parameters == null) ? Collections.EMPTY_MAP : parameters; > >> ((Configurable)tool).configure(params); > >> } > > >-0 i won't object to this, but i'll let you do the work. please > >update the relevant documentation as well. (Configurable's javadoc, > >index.xml for VelocityView, etc.) > > >> Best regards > >> Henning > >> > >> > >> > >> >Modified: > >> > > >> > jakarta/velocity/tools/trunk/src/java/org/apache/velocity/tools/view/ViewToolInfo.java > >> > >> >Modified: > >> >jakarta/velocity/tools/trunk/src/java/org/apache/velocity/tools/view/ViewToolInfo.java > >> >URL: > >> >http://svn.apache.org/viewcvs/jakarta/velocity/tools/trunk/src/java/org/apache/velocity/tools/view/ViewToolInfo.java?rev=321226&r1=321225&r2=321226&view=diff > >> >============================================================================== > >> >--- > >> >jakarta/velocity/tools/trunk/src/java/org/apache/velocity/tools/view/ViewToolInfo.java > >> > (original) > >> >+++ > >> >jakarta/velocity/tools/trunk/src/java/org/apache/velocity/tools/view/ViewToolInfo.java > >> > Fri Oct 14 14:54:13 2005 > >> >@@ -183,7 +183,7 @@ > >> > LOG.error("Exception while instantiating instance of \"" + > >> > getClassname() + "\": " + e); > >> > } > >> >- if (configurable) > >> >+ if (configurable && parameters != null) > >> > { > >> > ((Configurable)tool).configure(parameters); > >> > } > >> > >> > >> > >> >--------------------------------------------------------------------- > >> >To unsubscribe, e-mail: [EMAIL PROTECTED] > >> >For additional commands, e-mail: [EMAIL PROTECTED] > >> > >> -- > >> Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH > >> [EMAIL PROTECTED] +49 9131 50 654 0 http://www.intermeta.de/ > >> > >> RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire > >> Linux, Java, perl, Solaris -- Consulting, Training, Development > >> > >> 4 - 8 - 15 - 16 - 23 - 42 > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: [EMAIL PROTECTED] > >> For additional commands, e-mail: [EMAIL PROTECTED] > >> > >> > > >--------------------------------------------------------------------- > >To unsubscribe, e-mail: [EMAIL PROTECTED] > >For additional commands, e-mail: [EMAIL PROTECTED] > > > -- > Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH > [EMAIL PROTECTED] +49 9131 50 654 0 http://www.intermeta.de/ > > RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire > Linux, Java, perl, Solaris -- Consulting, Training, Development > > 4 - 8 - 15 - 16 - 23 - 42 > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
