Hi Victor 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 :)
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. 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. >> > >
