Re: Suspicious code

[Vadim Gritsenko <va...@reverycodes.com>]

Neil Cook wrote:

> 
> Original line:
>              node = node.getParentNode();
> 
> Suggestion:
>              node = next.getParentNode();
> 
> With that change, the traversal would first go through the deepest 
> descendant of the previous node, or the previous node itself, or the 
> parent node, in that order of choices. If all those choices return null, 
> that is returned. Choosing a non-null node, it must return true from 
> acceptNode(node) or the process will repeat.

Not exactly true :-)

With your suggestion, if parent node is not accepted, eternal loop will 
be created: same parent of the same next node will be tested again and 
again.

I will commit alternate fix - as soon as I test it :-)

Vadim


> /Neil
> 
> Vadim Gritsenko wrote:
> 
>> Dave Brosius wrote:
>>
>>> In org.apache.xindice.xml.dom.traversal.TreeWalkerImpl is this code
>>>  
>>> Clearly this is wrong:
>>>  
>>>          if ( node == null )
>>>             node = node.getParentNode();   
>>
>>
>> Ack. This code is still in there.
>>
>> Do you have patch suggestion? And if you can also provide a JUnit test 
>> for it, that would be superb :-)
>>
>> Vadim
>>
>>
>>>    public Node previousNode() {
>>>       Node node = next;
>>>       while ( true ) {
>>>          node = node.getPreviousSibling();
>>>          if ( node == null )
>>>             node = node.getParentNode();
>>>          else
>>>             while ( node.hasChildNodes() )
>>>                node = node.getLastChild();
>>>          if ( node != null && acceptNode(node) ) {
>>>             next = node;
>>>             return next;
>>>          }
>>>          else if ( node == null )
>>>             return null;
>>>       }
>>>    }