commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bugzi...@apache.org
Subject DO NOT REPLY [Bug 35096] - [Digester][PATCH] Cannot add multiple CallMethodRules for the same pattern
Date Tue, 31 May 2005 11:51:42 GMT
DO NOT REPLY TO THIS EMAIL, BUT PLEASE POST YOUR BUG·
RELATED COMMENTS THROUGH THE WEB INTERFACE AVAILABLE AT
<http://issues.apache.org/bugzilla/show_bug.cgi?id=35096>.
ANY REPLY MADE TO THIS MESSAGE WILL NOT BE COLLECTED AND·
INSERTED IN THE BUG DATABASE.

http://issues.apache.org/bugzilla/show_bug.cgi?id=35096





------- Additional Comments From skitching@apache.org  2005-05-31 13:51 -------
(In reply to comment #4)
> Thats one approach. I, personally, would never bother to download any extras 
> unless:

Yes, it is a problem to encourage people to use stuff not in the core distro.
But it's also bad to bundle stuff in the core distro before people have had a
good chance to use it and report issues :(.

We just don't have time to "demonstrate full backward compatibility". How
exactly can that be done anyway? The best we can do is bundle the functionality
either as an extra, or as part of a series of beta releases. Either way, only a
few people will use it so a lot of time is needed to get good feedback. 

> > And while it really will be necessary to somehow link CallParamRule objects 
> to
> > their associated CallMethodRule I don't think using integer indexes are the
> > solution. It just feels clumsy.
> 
> I'm moderately surprised, primarily, due to the existence of paramIndex. I had 
> a similar reaction when I saw paramIndex, since I don't specify where an 
> argument should go on the parametes stack when making a function call, I just 
> supply the arguments in the right order. So I wondered why that index exists, 
> and figured out (as I wrote some Digesters), that one may want to order the 
> addRules by patterns, and in complex schemas, there are times when you 
> encounter params 'out of order' as you traverse the document structure. 

It would be reasonable I guess for CallParamRule objects which are created
without explicit "parameter indexes" to assume they have been added to the
digester in parameter order. However it simply isn't done that way.

Parameter indexes are a reasonably "natural" way of describing parameters
anyway. It's quite normal for a programmer to talk about "the third parameter to
method X".

But having a "two dimensional" scheme where there is an index to describe which
CallMethodRule a ParamRule is associated with is quite different; it isn't part
of the problem ("the third param to method X"), it's an artefact of the digester
library's implementation. And that's what makes it a really awkward solution in
my opinion.


> My mistake, should have been more explicit in the TODO I added on top of the 
> instanceof. I do not want any special treatment for CallMethodRule, the rogue 
> bits in the core can be easily shifted to CallMethodRule. The purpose I put 
> them there, was to socialize those and get feedback of the form "hey, here's a 
> better way of figuring out this is the n'th callMethodRule added for pattern 
> foo/bar". We can put that bit in CallMethodRule, the setDigester isn't exactly 
> a side-effect free setter anyway.

Yes, it might be possible to compute that in CallMethodRule.setDigester by
scanning the Rules. But only maybe, because currently the Rules object can
return a list of the Rule objects added to it, but not the patterns associated
with those objects. So we're talking major surgery to the Rules class I think to
add this feature - and thereby breaking any user class that has implemented the
Rules interface. Unless there's some way CallMethodRule can figure this out
which I haven't seen.

[cut stuff re my "strawman" solution]
> 
> This can always be the default behavior when the callMethodRule index is not 
> specified. But again, I recommend that it be possible for a CallParamRule to 
> point to the CallMethodRule that it is refering too (I used index in document 
> order). If you buy paramIndex, this is a straight sell. Anytime we introduce a 
> default behavior which cannot be overridden, we're just digging ourselves 
> another hole.

Yes, that's true. The alternative I have considered is having ParamRule classes
take a pointer to a CallMethodRule object. But then the nice
factory-method-based approach of configuring Digester gets really clumsy. And I
have no idea how such an approach would be represented in xmlrules.

> Which brings me to a broad comment, based on my use of Digester, in its 
> current state, its very useful for flat schemas, other than that you end up 
> writing a lot of custom rules.

I've not found that, and I've used digester for some pretty complicated xml
processing.

> 
> I can propose a few iterations, if you wish, as and when I get some time.

Please feel free. I'll definitely have a look at any patches you propose.
Though if they use "call method rule indexes" to link param rules to their
method rule they probably won't get far with me :-)

You may also have seen that I have posted version 1.7 RC1 for review (and
hopefully release fairly soon). Solving this problem is not high enough priority
to hold back the release.

> Let me know if you want me to re-attach.

Don't worry. The file is downloadable, you just need to right-click on it rather
than left-click.

Regards,

Simon


-- 
Configure bugmail: http://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message