On 3 Dec 2003, at 22:46, Simon Kitching wrote:


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.

but that's the beauty of that class :)


(i'll expand later.)

<snip>

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.

it may be trivial but i have a strongly feeling that variation in this area will be required for many common use cases. for example, to substitute just attributes or to use a different system of variable encoding. by providing this trivial hook, the user only has to write the smallest possible amount of code (and they are required to learn only a small part of the design) in order to solve these common cases.


i've found that it's often better to best to provide both the right general solution and an easy way to solve the most common customization use cases. often, this may lead to duplicate hook points (in this case, Substitution and VariableExpander) but it makes things easy for users (and for those who explain to users how do use libraries ;)

- robert


--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to