For Loop iterated twice even when size of NodeList is 1

Question

I am facing some kind of a weird(as it appears to me) problem. I have a NodeList and need to remove an element from the NodeList while iterating through it. The NodeList has only one child element , so after removing that element that NodeList does not have any child element. Ideally the for loop should have stopped after removal of that element, but this is not happening and as the for loop runs for the second time, even when there are no child elements available, I am getting a NullPointerException.

Sample XML :

<Order OrderNo="1">
  <Lines>
    <Line LineNo="1"/>
  </Lines>
 </Order>

Sample Code :

    NodeList nlLine = inDoc.getElementsByTagName("Line");
    for(int cntLn = 0 ; cntLn < nlLine.getLength() ; cntLn++){

        Element elLn = (Element) nlLine.item(cntLn);

        if(//some condition){

        elLn.getParentNode().removeChild(elLn);
        cntLn--;

         }

    }

I am getting NullPointer exception on the linewhere i am getting the Parent Node and then removing the child. Any clue/lead/help on this?

Answer 1

You are decrementing the loop variable cntLn--. I am pretty sure you meant to increment it cntLn++. And since you are already incrementing, just omit the line cntLn--;

TIP: As a healthy coding practice, always use the prefix increment operator ++cntLn in case of loops (unless you explicitly need the postfix behavior).

Answer 2

I think this behavior is caused by the last line of your loop (cntLn--;).

Assume, the list has one child, so you start your processing with cntln=0 and length=1 which means: execute the loop. Then you remove the node since the condition is evaluated to true and you decrease the counter to -1. After that your for-loop increases cntln again to 0 and you start again but now the node list of the Lines-element is empty!

Simply omit the last line and try again ;)

Nevertheless, I think this is a critical processing since you cannot foresee side-effects when removing nodes while looping over the node list. Maybe it would be better to store the index of each node to be removed in this loop and remove them later in another loop.

Answer 3

Let's see what your program does.

1. Getting Line nodes (nlLine)
2. Setting the cycle variable to 0, since it's bigger than nlLine's size (1), we are going in the cycle.
3. Supposing the condition is true
4. Removing a node from XML (nlLine not updated!)
5. decrement the cycle variable
6. Cycle variable is still 0 (because of step 5.) and the nlLine hasn't been updated so we are going in to the cycle again (0 < 1)
7. Since elLn did not have a parent at this time NPE will be thrown (it has been removed from the XML)

Solution: leave cntLn--; out from your code. You have to step to the next element of the nodelist, since elLn.getParentNode().removeChild(elLn); won't remove your actual node from the nodeList.

Tip: usually it's a bad practice to modify the list during for cycling over it. Although it's a bit different situation, but still try to avoid. You should do it AFTER the main loop. (e.g. create a removeThese list to store the elements you want to delete from the list you are looping, and you can remove them once you are done with the main loop. It's safe, it's easy to understand and easy to manage.)

Answer 4

Simply increment and not decrement cntLn by one when removing the child - the upper bound of the for loop is static, so your only chance to manage the state of the for loop is to modify the loop variable.