commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From robert burrell donkin <robertburrelldon...@blueyonder.co.uk>
Subject Re: [digester] Proposal to change Rules interface to return ListIterator for matches
Date Mon, 29 Sep 2003 10:14:56 GMT

On Sunday, September 28, 2003, at 11:38 PM, Simon Kitching wrote:

> On Mon, 2003-09-29 at 03:40, robert burrell donkin wrote:
>> On Saturday, September 27, 2003, at 06:43 PM, Craig R. McClanahan wrote:
>>
>>> Simon Kitching wrote:
>>>
>>>> Hi,
>>>>
>>>> The current Rules interface defines
>>>>
>>>>  public List match(String uri, String patern);
>>>>
>>>> This is giving me significant problems for the Plugins module I am
>>>> working on. I also think it is simply the wrong type to be returning
>>>> here; a List has many operations available on it which are not
>>>> appropriate in this situation.
>>>>
>>>> I would like to suggest that the following method be added to the 
>>>> Rules
>>>> interface, and the "matches" methods be deprecated:
>>>>
>>>>  public ListIterator iterator(String uri, String pattern);
>
>>>>
>>>> This seems to me to encapsulate the goal of the existing "matches"
>>>> method: to provide access to the set of Rule objects matching the
>>>> criteria. [I'm flexible on the method name ;-]
>>>>
>>> I'm OK with the concept of returning an iterator from a Rules
>>> implementation, but would suggest (if we do it) that the signature say
>>> Iterator instead of ListIterator (the implementation in RulesBase can, 
>>> of
>>> course, return a ListIterator if it wants to).
>>>
>>> We'd certainly need to keep the existing match() signature around for
>>> backwards compatibility.  Adding methods to interfaces is also a nasty
>>> thing to do, but I suspect most people will be extending RulesBase
>>> already.
>>
>> i'm not so sure about that. when i developed the RegexRules i discovered
>> that when creating a very different implementation, RulesBase is
>> unsuitable. (that's why i added AbstractRulesImpl.)
>>
>>
>> if we are thinking about changing Rules yet again then it seems to me 
>> that
>> we should consider whether making this class an interface was the correct
>> design decision in the first place. if Rules had been an abstract class
>> then it would have been very easy to make the change suggested by simon
>> without breaking compatibility. maybe it should be an abstract class.
>>
>> we could think about deprecating Rules in favour of an abstract class
>> (AbstractMatchingRules or something). getRules and setRules would be
>> deprecated but would continue to be supported via wrappers. we could add
>> getMatchingRules and setMatchingRules methods which take instances of the
>> abstract class.
>>
>> i'd prefer to go down this route (if possible) since it would not only
>> preserve backward compatibility but also allow more flexible 
>> modifications
>> to be made in the future.
>>
>> - robert
>
> Well, I just want to note that the original reason I raised the
> possibility of returning ListIterator for matches was to resolve a
> problem I had with implementing a Plugins module. I have since had an
> "epiphany" :-) and have found a promising solution to my problem that
> hopefully will not require any change to the way Digester works.
>
> So from my point of view, I hopefully don't *need* any change to
> Digester at the moment.

still, you original reasoning still seems pretty valid. it seems to me 
that often when a design seems too inflexible in a particular direction, 
it's better to fix it immediately rather than wait until solutions to 
other similar issues are also blocked by this inflexibility.

> FYI: the problem I am struggling with is being able to add rules to the
> set of "matches" for a particular element after Digester's startElement
> method has begun iterating over the set of matching rules. eg given that
> pattern "foo/bar" matches 2 rules: a PluginCreateRule, and a
> SetNextRule, when the PluginCreateRule fires, new rules such as a
> SetPropertiesRule may need to be added to the rules associated with that
> same "foo/bar" pattern. The possible solution I have come up with is to
> *not* add rules matching the current pattern to the Digester, but
> instead have the PluginCreateRule object record these rules, and fire
> them from its begin(), body() and end() methods.

IMHO adding rules dynamically is best avoid where possible. experience 
with betwixt has shown that it's difficult to mix in custom rules when 
other rules add rules dynamically. (the main issue is that preserving the 
order.)

the latest solution being using on betwixt is that a single rule will be 
used and then delegation used to manage dynamic mappings. it sounds like 
you've managed to hit upon what is (IMHO) the best way to solve this 
problem more quickly than we did for betwixt.  i'd support the creation of 
an abstract delegator rule that allows others to easily understand and use 
this pattern if you go down this route.

- robert


Mime
View raw message