Hi Julian, > I agree that it's not worthwhile adding an accept() method to the > Resource API, especially since the benefit of double-dispatch, i.e. > visiting different types without casting, isn't much help in a world > of uniform Resources. We know we're only visiting Resources and there > are pretty much only two meaningful ways of traversing a tree: > breadth-first and depth-first. So the utility in the Visitor you're > creating is in the pre-baked traversal logic, and I guess that should > be the focus. Having that logic in an abstract Visitor class seems > fine to me. It might be interesting to look at the implementation of > FilteringItemVistor[0]. I remember looking at this class a long time > ago and I remember that I thought the includePredicate and > traversalPredicate were pretty neat and useful. However, currently > their utility evade me :)
what the predicates allow you to do is to skip traversal of certain parts of the resource tree. The ResourceVisitor doesn't allow this of course, but I think that's a rather rare requirement and could be easily added in a subclass. > > One minor nitpick: unless I am mistaken, the traversal logic should be > in accept() and the logic implemented by the API's user in the visit() > method. Yeah, you're right. I always confuse those two, mostly because I never really got the deeper meaning of their naming. Ciao, Victor. > > Regards > Julian > > [0] > http://jackrabbit.apache.org/api/2.2/org/apache/jackrabbit/commons/visitor/FilteringItemVisitor.html > > > > On Mon, Aug 22, 2011 at 10:56 PM, Victor Saar <[email protected]> wrote: >> Hi, >> >> thinking about this again: I agree that it is good to adhere to known >> approaches (and it's actually the GoF way of doing it, which is well known), >> but it has one disadvantage. It would require to change the Resource >> interface to add an accept(ResourceVisitor) method. Keeping the traversal >> code in the visitor itself though, has the advantage that Resource remains >> unchanged and traversal can be changed by classes extending ResourceVisitor. >> Here's how a simple implementation could look like: >> >> /** >> * The <code>ResourceVisitor</code> helps in traversing a resource >> * tree by decoupling the actual traversal code from application code. >> * Concrete subclasses should implement the >> * {@link ResourceVisitor#accept(Resource)} method. >> */ >> public abstract class ResourceVisitor { >> >> /** >> * Visit the given resource and all its descendants. >> * @param res The resource >> */ >> public void visit(Resource res) { >> if(res != null) { >> accept(res); >> >> traverseChildren(res.listChildren()); >> } >> } >> >> /** >> * Visit the given resources and all its descendants. >> * @param children The list of resources >> */ >> protected void traverseChildren(Iterator<Resource> children) { >> while(children.hasNext()) { >> Resource child = children.next(); >> >> accept(child); >> >> traverseChildren(child.listChildren()); >> } >> } >> >> /** >> * Implement this method to do actual work on the resources. >> * @param res The resource >> */ >> protected abstract void accept(Resource res); >> } >> >> Feedback is welcome. >> >> Ciao, Victor. >> >> On 19.08.2011, at 02:33, Victor Saar wrote: >> >>> Hi Justin, >>> >>> thanks for your feedback. As soon as I find some time, I'll create an issue >>> for that and attach a patch for review. >>> >>> Ciao, Victor. >>> >>> On 18.08.2011, at 19:13, Justin Edelson wrote: >>> >>>> I'd love to see a patch for this :) >>>> >>>> I would suggest modeling the implementation on the JCR >>>> ItemVisitor/TraversingItemVisitor classes as that is likely to be >>>> something which at least some potential users are familiar. >>>> >>>> Justin >>>> >>>> On Thu, Aug 18, 2011 at 2:51 AM, Victor Saar <[email protected]> wrote: >>>>> Hi, >>>>> >>>>> from time to time I have the need to walk a resource tree to do something >>>>> with a certain type of resource. I've written a very basic >>>>> ResourceVisitor abstract class where I can implement an accept() method. >>>>> This effectively decouples the traversal code from my application code. >>>>> >>>>> As I think this is a pretty common use case, I wanted to ask if it makes >>>>> sense to add such a helper class to Sling itself? Maybe with some added >>>>> functionality to stop traversing when a certain resource is found. WDYT? >>>>> >>>>> Ciao, Victor. >>> >> >>
