On Fri, 2003-12-05 at 11:17, Simon Kitching wrote: > Hi Robert, > > I think the code committed is just fine.
On further thought, I have a more significant change to suggest. I think the VariableSubstitutor and MultiVariableExpander classes should be merged. VariableSubstitutor really is a nothing class, just delegating all its work to other classes. If it is decided to have just one implementation that handles both the simple ant-like case, and the more complex implementation currently in MultiVariableExpander, then the resulting class name could be "BaseVariableSubstitutor", otherwise it could be called MultiVariableSubstitutor. I suggest that the (new) BaseVariableSubstitutor should implement both Substitutor and VariableExpander, and pass itself to the VariableAttributes class as the expander to use. This does mean that the constructor would need to change: public BaseVariableSubstitutor(boolean doAttributes, boolean doBody) PRO: * one less class in the digester code base * one less class for the user to deal with. they just instantiate a BaseVariableSubstitutor (or whatever) rather than both a VariableSubstitutor and a MultiVariableExpander. CON: A user can't just write a new VariableExpander implementation; they need to write a new Substitutor implementation instead. However as noted above, the code currently in VariableSubstitutor is pretty trivial; I can't see that being an issue in practice. Regards, Simon --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
