[EMAIL PROTECTED] wrote:
Greetings,

While working with the NodeCachingLinkedList, I seem to have found a bug in
the implementation of addLast().  Basically, it has to do with the fact that
CommonsLinkedList.createNode() has the parameters named "next", "previous",
"element", but the constructor for Node() names the parameters "previous",
"next", "element".  In addNodeBefore(), it passes the parameters in the
order "previous", "next", "element".  This works most of the time because of
the way createNode passes the paramters to the Node constructor.  However
NodeCachingLinkedList gets them backwards when it populates the cached node.

The following patch fixes the probably temporarily, but I suspect the real
fix is to make sure that createNode and the Node constructor and all calling
methods are consistent:

diff -w -u -r1.7 NodeCachingLinkedList.java
--- NodeCachingLinkedList.java  31 Aug 2003 17:26:44 -0000      1.7
+++ NodeCachingLinkedList.java  3 Oct 2003 23:30:39 -0000
@@ -212,8 +212,8 @@
         if (cachedNode == null) {
             return super.createNode(next, previous, element);
         } else {
-            cachedNode.next = next;
-            cachedNode.previous = previous;
+            cachedNode.next = previous;
+            cachedNode.previous = next;
             cachedNode.element = element;
             return cachedNode;
         }

Here's a test case that exposes the problem:

    public void testRemoveLast()
    {
        NodeCachingLinkedList list = new NodeCachingLinkedList();
        list.addAll( Arrays.asList( new String[]{"value1", "value2"}));
        assertEquals( "value1", list.removeFirst() );
        list.addLast( "value3");
        assertEquals( "value2", list.removeFirst() );
        assertEquals( "value3", list.removeFirst() );
        list.addLast( "value4" );
        assertEquals( "value4", list.removeFirst() );
    }

What happens is that the final "removeFirst" throws a NullPointerException
without the patch above.

Let me know if anyone has questions!

Thanks! This is definitely a problem, which should be addressed by making the parameter order consistent, as you describe. Thanks for pointing this out.


Phil


David CONFIDENTIALITY NOTICE This electronic mail transmission and any accompanying documents contain information belonging to the sender ("Information") that may be confidential and legally privileged. If you are not the intended recipient, any disclosure, copying,distribution or action taken in reliance on the Information is strictly prohibited. If you have received the Information in error,please contact the sender by reply email and destroy all copies of the original email. Thank you. �




---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to