I would like us to reconsider the use of the VariableResolver interface for VariableFormatter.
It seems that this is a cleaner design that does not force subclassing or composition as the sole mean of feature extension. Considering the complexity of the StrTokenizer class, I do not think that the earlier concern that VariableFormatter+VariableResolver as too framework-like is really valid. The interface VariableResolver could be made to live in the VariableFormatter class we *really* think we need to "hide" this feature. If Oliver is up for it and the list does not say "no, no, because...", I'd like to see a patch to the CVS code that makes VariableFormatter use a VariableResolver with an canned implementation for Maps. Gary > -----Original Message----- > From: Oliver Heger [mailto:[EMAIL PROTECTED] > Sent: Saturday, July 09, 2005 2:38 AM > To: Jakarta Commons Developers List > Subject: Re: [lang] text.Interpolation, on to 2.2 > > Some comments inline... > > Stephen Colebourne wrote: > > > Gary Gregory wrote: > > > >>> The source of values for the VariableFormat class is only allowed to > be > >>> a Map. I'm not sure if people will want other types of data sources. > >> > >> > >>> Yes the resolveVariable method does allow customisation via > subclassing > >> > > >>but the fact that the "default" source is a map is very obviously > > exposed > > > >>> via the class API. > >> > >> > >> Well, should we go back to the interface approach? > > > > > > The constrast is with the StrTokenizer class. That alread has an > > interface, and various implementations of the interface. Thus an > > interface for this class is not inapropriate. > > > > I was hoping that there might have been some way to take the > > StrTokenizer interface and make it top level and reuse it in all the > > classes in the text package. Perhaps for locating the delimiters in > > VariableFormatter. But I'm not sure that idea works. > > > > > > Considering the current VariableFormatter class: > > > > a) we don't have the ability to call it directly from StrBuilder > > without copying the char array to a String. (VariableFormatter needs > > rewriting to operate on a char[]) > > Okay, to remain consistency this point sounds reasonable. If > VariableFormatter is reworked to operate on a char[], StrTokenizer will > certainly be useful. I think, it shouldn't be a problem to implement the > Matcher interface to locate variables in the source string. But I will > have to study the StrTokenizer class whether it is possible to extract > the variables' names once they have been detected. > > > > > b) we have lost the ability to have multiple substitutions > > I don't understand what you mean with this point. > > > > > c) we have added a complex escaping mechanism > > That's true, escaping increases complexity. I think we will have to find > a consense how far we want to go in this area. If you are in control > over the input for VariableFormatter, escaping might not be an issue > because you can choose variable delimiters that do not conflict with the > output you want to generate. But if this is not the case, the use case > of escaping variables will sooner or later come in naturally. > > > > > d) we don't have a replaceOnce() method > > This could be realized by passing a noRecursive flag to the main > substitution method; if this flag is set, no recursive calls will be made. > > > > > I would hope that we could have a static method, that is passed all > > the state, and performs all the work. That way it can be called from > > StrBuilder in the most efficient way. And StrTokenizer should be > > rewritten like this too. > > This is a good point. The main replace method could be implemented as a > static method, but we would then lose the possibility of overloading the > resolveVariable() method to extend the provided functionality. So in > this case I would prefer to have a VariableResolver interface (instead > of a plain map), which will support customization more easily. > > > > > Stephen > > > Oliver > > --------------------------------------------------------------------- > 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]
