commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ASHWIN Suresh <a.sur...@sigmatel.net.ma>
Subject RE: [digester] variable expansion
Date Wed, 03 Dec 2003 09:59:24 GMT
Sorry to jump in to this thread this way, and perhaps it is too late now.
But, have the people here considered using the term "resolve"
for this concept?

Perhaps the interface could be named Resolver, with the method resolve().

I can think of ${foo} > xyz as resolving the definition rather than simple
substitution,
thow at a lower level of abstraction, it is substitution.

If this has been considered and vetoed, please ignore my email.

One more point:

> The spelling "substituter" feels more natural to me than 
> "substitutor".
> 
> cf.:
> to write --> writer
> to drive --> driver
> to expand --> expander


For Latinate words, the pattern is usually -or.
Constructor, translator, delegator, etc.
Whenever, the agent form is formed out of removal of -ion, the preferred
suffix is -or.
Thus, the more appropriate form is substitutor.
Again, perhaps this was already discussed.

Ash





> -----Original Message-----
> From: Simon Kitching [mailto:s_kitching@paradise.net.nz]
> Sent: Thursday, December 04, 2003 09:18
> To: commons-dev@jakarta.apache.org
> Cc: simon@ecnetwork.co.nz
> Subject: [digester] variable expansion
> 
> 
> Hi Robert,
> 
> I think the code committed is just fine.
> 
> I like the versatile "Substitutor" interface; I can imagine 
> all sorts of
> uses for the ability to arbitrarily manipulate attribute 
> values and body
> text before rules see them other than just var expansion.
> 
> I also like the separation of this code out into a package, with only
> the Substitutor interface in the main code.
> 
> Some *very* minor comments: 
> 
> --
> 
> In VariableSubstitutor, you use lazy creation for variableAttributes.
> I'm not sure there is much point to that. Given that the user 
> has set up
> var expansion in the first place, the object will almost certainly be
> created sometime. However the existing code has to evaluate an
> if-statement on each call to determine if the object already exists. I
> think it also makes the code more complex than it needs to be. Why not
> create it in the constructor as soon as it is known if there is an
> attributeExpander or not?
> 
> --
> 
> I think a comment is needed in the Substitutor "interface" specifying
> that Digester guarantees never to hold the return value of the
> substitute(Attributes) method past the next call to the method. It is
> this behaviour that allows a single VariableAttributes object to be
> reused by the VariableSubstitutor. This behaviour was obvious before,
> when VariableAttributes was directly invoked from the Digester's
> startElement method, but probably needs to be explicitly 
> documented now.
> 
> --
> 
> You mention that you were considering whether it is necessary to have
> Remy's VarExpander implementation as well as the 
> MultiVariableExpander.
> While it would be a shame to discard Remy's work, I don't see much
> benefit from having an additional implementation.
> 
> The MultiVariableExpander does:
>         for(int i=0; i<nEntries; ++i) {
>             param = expand(
>                 param, 
>                 (String) markers.get(i), 
>                 (Map) sources.get(i));
>         }
>         return param;
> 
> The call to "expand" in the loop does exactly the same work as an
> implementation that supports only "${foo}" would do.
> 
> The overhead of looping once (ie nEntries=1) is trivial. The 
> overhead of
> fetching the marker and map from ArrayLists and casting them 
> is slightly
> higher, but still pretty small I think. That's the only performance
> difference I can see from an implementation customised for ant-like
> variables only. Oh - and avoiding one string concatenation in 
> the expand
> method ("$" + "{").
> 
> The API would be *very* slightly simpler: setSource(map) rather than
> addSource("$", map). I think this is more than offset by the 
> complexity
> of having multiple VarExpander implementations to choose from.
> 
> If the consensus is that the MultiVariableExpander is ok to 
> use for the
> simple case too, then it would probably make sense to rename it (eg
> BaseVariableExpander).
> 
> --
> 
> The spelling "substituter" feels more natural to me than 
> "substitutor".
> 
> cf.:
> to write --> writer
> to drive --> driver
> to expand --> expander
> 
> 
> Cheers,
> 
> Simon
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message