Details

Type: Improvement

Status:Resolved

Priority: Major

Resolution:
Won't Fix

Affects Version/s:
None

Fix Version/s:
None

Component/s:
None

Labels:

None

Description

I never really liked that we need inner classes (implementations of IVisitor) for traversing the component hierarchy. When needed, it comes easily to me that I need MarkupContainer.visitChildren(), but I always need to look up how to use it and friends. Debugging is also more complicated than it has to (breakpoint inside the visit function: either another breakpoint behind visitChildren or "press the continue-until-end-of-method" key several times until you are back). For these reasons I gave Iterators another try. I've attached a patch for others to review and provide feedback. I've also added test cases (ComponentIteratorTest) and I changed quite some visitChildren occassions in core. Maven compiles and successfully executes all tests.

I put the new classes in org.apache.wicket.util.iterator
ComponentIterator - An enhanced iterator with filters. Also supports chaining iterators. Builder API for class filters, isvisible filters, isenabled filters etc. Supports Java for each.
ComponentHierarchyIterator - enhances ComponentIterator to provide hierarchy traversal. Adds traversal filters to separate traversal control from what next() returns. Same Builder API. Supports Java for each.
IteratorFilter - Simple abstract class to implement the filter conditions. The Builder API makes use of it to add filters to the iterator.

Some examples:
ComponentHierarchyIterator iter = new ComponentHierarchyIterator(page);
while (iter.hasNext())

Activity

+1
I like IVisitor because it is a very natural way of visiting children and enable a good decoupling between callback and visiting logic. i.e. you can place both in different classes and extend/reuse them.
But the majority of use cases are simple enough to demand a new type, and the proposed idea address them nicely.

Why IteratorFilter#onFilter has the leaf parameter? Seems more natural to have Component#isLeaf instead a polluted API for filters. Most of filters I saw in the patch ignore this flag.

Pedro Santos
added a comment - 13/Jun/11 15:13 +1
I like IVisitor because it is a very natural way of visiting children and enable a good decoupling between callback and visiting logic. i.e. you can place both in different classes and extend/reuse them.
But the majority of use cases are simple enough to demand a new type, and the proposed idea address them nicely.
Why IteratorFilter#onFilter has the leaf parameter? Seems more natural to have Component#isLeaf instead a polluted API for filters. Most of filters I saw in the patch ignore this flag.

Igor Vaynberg
added a comment - 13/Jun/11 15:54 i do like this idea as well. it needs to be refined as it is still somewhat rough. eg i would like to be able to say
for (Foo foo:new ComponentHierarchyIterator<Foo>(page, Foo.class)) without having to cast to Foo myself
i would also like a new ComponentHierarchyIterator().postOrder()
the part where it gets messier is that it does not separate the configuration of the visit from the visit so i can say weird stuff like this:
ComponentHierarchyIterator it=new ComponentHierarchyIterator();
for (Component c:it)
{
it.filterByClass(Bar.class); <== mutating the visit like that - its strange to be able to do this
}
in any case, we should put this off until 1.6. 1.5 is stable and we need to release it asap.

I committed an inital version based on a much nicer implementation, but it's not used yet anywhere.

That is now supported
for (Foo foo:new ComponentHierarchyIterator<Foo>(page, Foo.class))

what is meant by ComponentHierarchyIterator().postOrder() ???

>> the part where it gets messier is that it does not separate the configuration of the visit from the visit so i can say weird stuff like this:
That's not yet implemented. Though you are right on filterByClass(), I can imagine you want to enable/disable certain filters depending on some previous results.

Juergen Donnerstag
added a comment - 07/Jul/11 19:16 I committed an inital version based on a much nicer implementation, but it's not used yet anywhere.
That is now supported
for (Foo foo:new ComponentHierarchyIterator<Foo>(page, Foo.class))
what is meant by ComponentHierarchyIterator().postOrder() ???
>> the part where it gets messier is that it does not separate the configuration of the visit from the visit so i can say weird stuff like this:
That's not yet implemented. Though you are right on filterByClass(), I can imagine you want to enable/disable certain filters depending on some previous results.

postOrder() means i want to visit deepest children first. see Visisits#visitPostOrder() in trunk. we use this, for example, when processing form components because we want to process children of formcomponentpanels before the panels themselves.

Igor Vaynberg
added a comment - 07/Jul/11 20:00 postOrder() means i want to visit deepest children first. see Visisits#visitPostOrder() in trunk. we use this, for example, when processing form components because we want to process children of formcomponentpanels before the panels themselves.