You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Mike Pettypiece <Mi...@EMBARCADERO.COM> on 2004/04/20 23:18:02 UTC

[collections] AbstractLinkedList.Node

Hello All,
 
I'm currently trying to subclass AbstractLinkedList.  I've implemented
my own subclass of AbstractLinkedList.Node (and overrode createNode() to
return it) but run into a bit of a problem.  It seems that the object
encapsulated by Node (Node.value) is referenced directly throughout
AbstractLinkedList instead of using accessors such as get/setValue().  
 
This presents a bit of a problem for me as Node.value does not contain
anything in my implementation.  I'm dealing with many large objects
which could easily exceed the heap size of a JVM if they were all loaded
into memory.  What I'm doing is using the Turbine JCS to store these
objects in a LRUMemoryCache which overflows into a IndexDiskCache.
Ideally if Node had get/setValue() methods I could override them and do
a lookup in the JCS in my implementation.
 
Adding the get/set methods to Node would also make it more consistent
with AbstractHashedMap.HashEntry (the Map equivalent).  The downside
would be the (slight) overhead of a method call.
 
What do people think of this change?  I've included a patch that someone
with CVS commit access can review.
 
Cheers,
 
Mike
 
Index: org/apache/commons/collections/list/AbstractLinkedList.java
===================================================================
RCS file:
/home/cvspublic/jakarta-commons/collections/src/java/org/apache/commons/
collections/list/AbstractLinkedList.java,v
retrieving revision 1.8
diff -u -r1.8 AbstractLinkedList.java
--- org/apache/commons/collections/list/AbstractLinkedList.java 18 Feb
2004 01:12:26 -0000 1.8
+++ org/apache/commons/collections/list/AbstractLinkedList.java 20 Apr
2004 21:15:08 -0000
@@ -110,7 +110,7 @@
 
     public Object get(int index) {
         Node node = getNode(index, false);
-        return node.value;
+        return node.getValue();
     }
 
 
//----------------------------------------------------------------------
-
@@ -130,7 +130,7 @@
     public int indexOf(Object value) {
         int i = 0;
         for (Node node = header.next; node != header; node = node.next)
{
-            if (isEqualValue(node.value, value)) {
+            if (isEqualValue(node.getValue(), value)) {
                 return i;
             }
             i++;
@@ -141,7 +141,7 @@
     public int lastIndexOf(Object value) {
         int i = size - 1;
         for (Node node = header.previous; node != header; node =
node.previous) {
-            if (isEqualValue(node.value, value)) {
+            if (isEqualValue(node.getValue(), value)) {
                 return i;
             }
             i--;
@@ -177,7 +177,7 @@
         // Copy the values into the array
         int i = 0;
         for (Node node = header.next; node != header; node = node.next,
i++) {
-            array[i] = node.value;
+            array[i] = node.getValue();
         }
         // Set the value after the last value to null
         if (array.length > size) {
@@ -224,14 +224,14 @@
 
//----------------------------------------------------------------------
-
     public Object remove(int index) {
         Node node = getNode(index, false);
-        Object oldValue = node.value;
+        Object oldValue = node.getValue();
         removeNode(node);
         return oldValue;
     }
 
     public boolean remove(Object value) {
         for (Node node = header.next; node != header; node = node.next)
{
-            if (isEqualValue(node.value, value)) {
+            if (isEqualValue(node.getValue(), value)) {
                 removeNode(node);
                 return true;
             }
@@ -266,7 +266,7 @@
 
     public Object set(int index, Object value) {
         Node node = getNode(index, false);
-        Object oldValue = node.value;
+        Object oldValue = node.getValue();
         updateNode(node, value);
         return oldValue;
     }
@@ -281,7 +281,7 @@
         if (node == header) {
             throw new NoSuchElementException();
         }
-        return node.value;
+        return node.getValue();
     }
 
     public Object getLast() {
@@ -289,7 +289,7 @@
         if (node == header) {
             throw new NoSuchElementException();
         }
-        return node.value;
+        return node.getValue();
     }
 
     public boolean addFirst(Object o) {
@@ -307,7 +307,7 @@
         if (node == header) {
             throw new NoSuchElementException();
         }
-        Object oldValue = node.value;
+        Object oldValue = node.getValue();
         removeNode(node);
         return oldValue;
     }
@@ -317,7 +317,7 @@
         if (node == header) {
             throw new NoSuchElementException();
         }
-        Object oldValue = node.value;
+        Object oldValue = node.getValue();
         removeNode(node);
         return oldValue;
     }
@@ -399,7 +399,7 @@
      * @param value  new value of the node
      */
     protected void updateNode(Node node, Object value) {
-        node.value = value;
+        node.setValue(value);
     }
 
     /**
@@ -632,6 +632,17 @@
             this.next = next;
             this.value = value;
         }
+        
+        public Object getValue() {
+         return this.value;
+        }
+
+        public Object setValue(Object value) {
+            Object old = this.value;
+            this.value = value;
+            return old;
+        }
+
     }
 
 
//----------------------------------------------------------------------
-
@@ -723,7 +734,7 @@
                 throw new NoSuchElementException("No element at index "
+
                         nextIndex + ".");
             }
-            Object value = next.value;
+            Object value = next.getValue();
             current = next;
             next = next.next;
             nextIndex++;
@@ -740,7 +751,7 @@
                 throw new NoSuchElementException("Already at start of
list.");
             }
             next = next.previous;
-            Object value = next.value;
+            Object value = next.getValue();
             current = next;
             nextIndex--;
             return value;
@@ -762,10 +773,10 @@
             nextIndex--;
             expectedModCount++;
         }
-
+ 
         public void set(Object obj) {
             checkModCount();
-            getLastNodeReturned().value = obj;
+            getLastNodeReturned().setValue(obj);
         }
 
         public void add(Object obj) {


Re: [collections] AbstractLinkedList.Node

Posted by Stephen Colebourne <sc...@btopenworld.com>.
Change made. Next time could you please attach patches rather than embedding
them.

I have also added methods for getting and setting the next and previous
fields. However I have not change the rest of the code to use them as
performance may be a greater question.

Stephen

----- Original Message -----
From: "Mike Pettypiece" <Mi...@EMBARCADERO.COM>
I'm currently trying to subclass AbstractLinkedList.  I've implemented
my own subclass of AbstractLinkedList.Node (and overrode createNode() to
return it) but run into a bit of a problem.  It seems that the object
encapsulated by Node (Node.value) is referenced directly throughout
AbstractLinkedList instead of using accessors such as get/setValue().

This presents a bit of a problem for me as Node.value does not contain
anything in my implementation.  I'm dealing with many large objects
which could easily exceed the heap size of a JVM if they were all loaded
into memory.  What I'm doing is using the Turbine JCS to store these
objects in a LRUMemoryCache which overflows into a IndexDiskCache.
Ideally if Node had get/setValue() methods I could override them and do
a lookup in the JCS in my implementation.

Adding the get/set methods to Node would also make it more consistent
with AbstractHashedMap.HashEntry (the Map equivalent).  The downside
would be the (slight) overhead of a method call.

What do people think of this change?  I've included a patch that someone
with CVS commit access can review.

Cheers,

Mike

Index: org/apache/commons/collections/list/AbstractLinkedList.java
===================================================================
RCS file:
/home/cvspublic/jakarta-commons/collections/src/java/org/apache/commons/
collections/list/AbstractLinkedList.java,v
retrieving revision 1.8
diff -u -r1.8 AbstractLinkedList.java
--- org/apache/commons/collections/list/AbstractLinkedList.java 18 Feb
2004 01:12:26 -0000 1.8
+++ org/apache/commons/collections/list/AbstractLinkedList.java 20 Apr
2004 21:15:08 -0000
@@ -110,7 +110,7 @@

     public Object get(int index) {
         Node node = getNode(index, false);
-        return node.value;
+        return node.getValue();
     }


//----------------------------------------------------------------------
-
@@ -130,7 +130,7 @@
     public int indexOf(Object value) {
         int i = 0;
         for (Node node = header.next; node != header; node = node.next)
{
-            if (isEqualValue(node.value, value)) {
+            if (isEqualValue(node.getValue(), value)) {
                 return i;
             }
             i++;
@@ -141,7 +141,7 @@
     public int lastIndexOf(Object value) {
         int i = size - 1;
         for (Node node = header.previous; node != header; node =
node.previous) {
-            if (isEqualValue(node.value, value)) {
+            if (isEqualValue(node.getValue(), value)) {
                 return i;
             }
             i--;
@@ -177,7 +177,7 @@
         // Copy the values into the array
         int i = 0;
         for (Node node = header.next; node != header; node = node.next,
i++) {
-            array[i] = node.value;
+            array[i] = node.getValue();
         }
         // Set the value after the last value to null
         if (array.length > size) {
@@ -224,14 +224,14 @@

//----------------------------------------------------------------------
-
     public Object remove(int index) {
         Node node = getNode(index, false);
-        Object oldValue = node.value;
+        Object oldValue = node.getValue();
         removeNode(node);
         return oldValue;
     }

     public boolean remove(Object value) {
         for (Node node = header.next; node != header; node = node.next)
{
-            if (isEqualValue(node.value, value)) {
+            if (isEqualValue(node.getValue(), value)) {
                 removeNode(node);
                 return true;
             }
@@ -266,7 +266,7 @@

     public Object set(int index, Object value) {
         Node node = getNode(index, false);
-        Object oldValue = node.value;
+        Object oldValue = node.getValue();
         updateNode(node, value);
         return oldValue;
     }
@@ -281,7 +281,7 @@
         if (node == header) {
             throw new NoSuchElementException();
         }
-        return node.value;
+        return node.getValue();
     }

     public Object getLast() {
@@ -289,7 +289,7 @@
         if (node == header) {
             throw new NoSuchElementException();
         }
-        return node.value;
+        return node.getValue();
     }

     public boolean addFirst(Object o) {
@@ -307,7 +307,7 @@
         if (node == header) {
             throw new NoSuchElementException();
         }
-        Object oldValue = node.value;
+        Object oldValue = node.getValue();
         removeNode(node);
         return oldValue;
     }
@@ -317,7 +317,7 @@
         if (node == header) {
             throw new NoSuchElementException();
         }
-        Object oldValue = node.value;
+        Object oldValue = node.getValue();
         removeNode(node);
         return oldValue;
     }
@@ -399,7 +399,7 @@
      * @param value  new value of the node
      */
     protected void updateNode(Node node, Object value) {
-        node.value = value;
+        node.setValue(value);
     }

     /**
@@ -632,6 +632,17 @@
             this.next = next;
             this.value = value;
         }
+
+        public Object getValue() {
+         return this.value;
+        }
+
+        public Object setValue(Object value) {
+            Object old = this.value;
+            this.value = value;
+            return old;
+        }
+
     }


//----------------------------------------------------------------------
-
@@ -723,7 +734,7 @@
                 throw new NoSuchElementException("No element at index "
+
                         nextIndex + ".");
             }
-            Object value = next.value;
+            Object value = next.getValue();
             current = next;
             next = next.next;
             nextIndex++;
@@ -740,7 +751,7 @@
                 throw new NoSuchElementException("Already at start of
list.");
             }
             next = next.previous;
-            Object value = next.value;
+            Object value = next.getValue();
             current = next;
             nextIndex--;
             return value;
@@ -762,10 +773,10 @@
             nextIndex--;
             expectedModCount++;
         }
-
+
         public void set(Object obj) {
             checkModCount();
-            getLastNodeReturned().value = obj;
+            getLastNodeReturned().setValue(obj);
         }

         public void add(Object obj) {




---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org