On 4 Dec 2003, at 09:17, Simon Kitching wrote:
<snip>
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?
you're probably right. they'd be a small saving if a null expander was passed in but we're probably better trading that for code clarity.
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.
yep. it's standard SAX behaviour but you're right about adding some javadocs for this.
i should probably go through and add some extra javadocs explanations to the Rule methods
saying that the Attributes may be recycled and should not be changed (but that'll have to wait till tomorrow).
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.
'm going to look into this. tomcat's an important user for digester and even if the differences are very minor (or of arguable value), i'd probably be inclined to include both. i do think that probably your implementation provides everything that remy's does but i want to take some time to analyze it properly.
- robert
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
