harmony-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hinde...@apache.org
Subject svn commit: r984789 - in /harmony/enhanced/java/branches/java6: ./ classlib/ classlib/depends/libs/ classlib/modules/luni/src/main/java/java/util/ArrayList.java drlvm/ jdktools/
Date Thu, 12 Aug 2010 14:06:31 GMT
Author: hindessm
Date: Thu Aug 12 14:06:31 2010
New Revision: 984789

URL: http://svn.apache.org/viewvc?rev=984789&view=rev
Log:
Merge change from /harmony/enhanced/java/trunk@984779:

  r984779 | hindessm | 2010-08-12 14:53:35 +0100 (Thu, 12 Aug 2010) | 3 lines
  
  Remove some REVIEW comments and minor cleanup after discussion with
  Oliver.
  


Modified:
    harmony/enhanced/java/branches/java6/   (props changed)
    harmony/enhanced/java/branches/java6/classlib/   (props changed)
    harmony/enhanced/java/branches/java6/classlib/depends/libs/   (props changed)
    harmony/enhanced/java/branches/java6/classlib/modules/luni/src/main/java/java/util/ArrayList.java
    harmony/enhanced/java/branches/java6/drlvm/   (props changed)
    harmony/enhanced/java/branches/java6/jdktools/   (props changed)

Propchange: harmony/enhanced/java/branches/java6/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Aug 12 14:06:31 2010
@@ -1,4 +1,4 @@
 /harmony/enhanced/java/branches/mrh:935751-941490
-/harmony/enhanced/java/trunk:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731
+/harmony/enhanced/java/trunk:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731,984779
 /harmony/enhanced/trunk:476395-929252
 /incubator/harmony/enhanced/trunk:292550-476394

Propchange: harmony/enhanced/java/branches/java6/classlib/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Aug 12 14:06:31 2010
@@ -1,7 +1,7 @@
 /harmony/enhanced/classlib/trunk:713674-735919,765923-926091,926318-926838
 /harmony/enhanced/classlib/trunk/working_classlib:884014-884286
 /harmony/enhanced/java/branches/mrh/classlib:935751-941490
-/harmony/enhanced/java/trunk/classlib:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731
+/harmony/enhanced/java/trunk/classlib:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731,984779
 /harmony/enhanced/trunk/classlib:476395-929252
 /harmony/enhanced/trunk/working_classlib:476396-920147
 /incubator/harmony/enhanced/trunk/classlib:292550-476394

Propchange: harmony/enhanced/java/branches/java6/classlib/depends/libs/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Aug 12 14:06:31 2010
@@ -1,4 +1,4 @@
 /harmony/enhanced/classlib/trunk/depends/libs:544451-926091
-/harmony/enhanced/java/trunk/classlib/depends/libs:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731
+/harmony/enhanced/java/trunk/classlib/depends/libs:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731,984779
 /harmony/enhanced/trunk/classlib/depends/libs:476395-929252
 /incubator/harmony/enhanced/trunk/classlib/depends/libs:292550-476394

Modified: harmony/enhanced/java/branches/java6/classlib/modules/luni/src/main/java/java/util/ArrayList.java
URL: http://svn.apache.org/viewvc/harmony/enhanced/java/branches/java6/classlib/modules/luni/src/main/java/java/util/ArrayList.java?rev=984789&r1=984788&r2=984789&view=diff
==============================================================================
--- harmony/enhanced/java/branches/java6/classlib/modules/luni/src/main/java/java/util/ArrayList.java
(original)
+++ harmony/enhanced/java/branches/java6/classlib/modules/luni/src/main/java/java/util/ArrayList.java
Thu Aug 12 14:06:31 2010
@@ -120,7 +120,6 @@ public class ArrayList<E> extends Abstra
             }
             array[--firstIndex] = object;
         } else if (location == size) {
-            // REVIEW: Why not just call add()? Matching RI behaviour?
             if (firstIndex + size == array.length) {
                 growAtEnd(1);
             }
@@ -128,10 +127,8 @@ public class ArrayList<E> extends Abstra
         } else { // must be case: (0 < location && location < size)
             if (size == array.length) {
                 growForInsert(location, 1);
-            } else if ((location < size / 2 && firstIndex > 0)
-                    || firstIndex + size == array.length) {
-                // REVIEW: Why not evaluate (lastIndex==array.length)
-                //         first to save the divide?
+            } else if (firstIndex + size == array.length
+                    || (firstIndex > 0 && location < size / 2)) {
                 System.arraycopy(array, firstIndex, array, --firstIndex,
                         location);
             } else {
@@ -180,8 +177,6 @@ public class ArrayList<E> extends Abstra
      */
     @Override
     public boolean addAll(int location, Collection<? extends E> collection) {
-        // REVIEW: Inconsistent exception case with
-        //         add(location,object) method
         if (location < 0 || location > size) {
             throw new IndexOutOfBoundsException(
                     // luni.0A=Index: {0}, Size: {1}
@@ -198,30 +193,19 @@ public class ArrayList<E> extends Abstra
             return false;
         }
 
-        // REVIEW: we use array.length - growSize in 3 places -
-        //         precalculate it?
         if (location == 0) {
             growAtFront(growSize);
             firstIndex -= growSize;
         } else if (location == size) {
-            // REVIEW: Don't need the above check as it can be no
-            //         other case. Make it just an else.
             if (firstIndex + size > array.length - growSize) {
                 growAtEnd(growSize);
             }
         } else { // must be case: (0 < location && location < size)
             if (array.length - size < growSize) {
-                // REVIEW: why grow growSize? Does growForInsert()
-                //         check we don't allocate too much?
                 growForInsert(location, growSize);
-            } else if ((location < size / 2 && firstIndex > 0)
-                    || firstIndex + size > array.length - growSize) {
-                // REVIEW: If condition above could be switched so
-                //         divide is done 2nd, same as add()
+            } else if (firstIndex + size > array.length - growSize
+                       || (firstIndex > 0 && location < size / 2)) {
                 int newFirst = firstIndex - growSize;
-                // REVIEW: Have we optimised for the case where there
-                //         is enough space at the end for growSize
-                //         rather than doing 2 array copies?
                 if (newFirst < 0) {
                     int index = location + firstIndex;
                     System.arraycopy(array, index, array, index - newFirst,
@@ -259,8 +243,6 @@ public class ArrayList<E> extends Abstra
             return false;
         }
         if (dumpArray.length > array.length - (firstIndex + size)) {
-            // REVIEW: why grow dumpArray.length rather than
-            //         dumpArray.length - free space?
             growAtEnd(dumpArray.length);
         }
         System.arraycopy(dumpArray, 0, this.array, firstIndex + size,
@@ -369,18 +351,12 @@ public class ArrayList<E> extends Abstra
     }
 
     private void growAtEnd(int required) {
-        int lastIndex = firstIndex + size;
-        // REVIEW: Isn't this next condition just:
-        //         required < array.length - size ?
-        if (firstIndex >= required - (array.length - lastIndex)) {
-            // REVIEW: Should use size! We don't seem to need newLast
-            //         - just use size calculated above
-            int newLast = lastIndex - firstIndex;
-            // REVIEW: Why not just a !=0 check here? And size cannot
-            //         be 0 unless required is 0, which does not happen
-            if (size > 0) {
+        if (array.length - size >= required) {
+            // REVIEW: as growAtEnd, why not move size == 0 out as
+            //         special case
+            if (size != 0) {
                 System.arraycopy(array, firstIndex, array, 0, size);
-                int start = newLast < firstIndex ? firstIndex : newLast;
+                int start = size < firstIndex ? firstIndex : size;
                 // REVIEW: I think we null too much
                 //         array.length should be lastIndex ?
                 Arrays.fill(array, start, array.length, null);
@@ -397,9 +373,7 @@ public class ArrayList<E> extends Abstra
                 increment = 12;
             }
             E[] newArray = newElementArray(size + increment);
-            // REVIEW: Just check size !=0? same as earlier check -
-            //         can be pulled out to earlier in the method?
-            if (size > 0) {
+            if (size != 0) {
                 System.arraycopy(array, firstIndex, newArray, 0, size);
                 firstIndex = 0;
             }
@@ -412,10 +386,10 @@ public class ArrayList<E> extends Abstra
             int newFirst = array.length - size;
             // REVIEW: as growAtEnd, why not move size == 0 out as
             //         special case
-            if (size > 0) {
+            if (size != 0) {
                 System.arraycopy(array, firstIndex, array, newFirst, size);
-                int length = firstIndex + size > newFirst ? newFirst
-                        : firstIndex + size;
+                int lastIndex = firstIndex + size;
+                int length = lastIndex > newFirst ? newFirst : lastIndex;
                 Arrays.fill(array, firstIndex, length, null);
             }
             firstIndex = newFirst;
@@ -428,7 +402,7 @@ public class ArrayList<E> extends Abstra
                 increment = 12;
             }
             E[] newArray = newElementArray(size + increment);
-            if (size > 0) {
+            if (size != 0) {
                 System.arraycopy(array, firstIndex, newArray, newArray.length
                         - size, size);
             }
@@ -525,16 +499,13 @@ public class ArrayList<E> extends Abstra
                             Integer.valueOf(location),
                             Integer.valueOf(size)));
         }
-        if (location == size - 1) {
+        if (location == 0) {
+            result = array[firstIndex];
+            array[firstIndex++] = null;
+        } else if (location == size - 1) {
             int lastIndex = firstIndex + size - 1;
             result = array[lastIndex];
             array[lastIndex] = null;
-        } else if (location == 0) {
-            // REVIEW: this if test is simpler why isn't it first?
-            //         (this moved up one place during the exception
-            //         change but it still probably should move up one more)
-            result = array[firstIndex];
-            array[firstIndex++] = null;
         } else {
             int elementIndex = firstIndex + location;
             result = array[elementIndex];
@@ -548,10 +519,10 @@ public class ArrayList<E> extends Abstra
                 array[firstIndex+size-1] = null;
             }
         }
-        // REVIEW: we can move this to the first if case since it
-        //         can only occur when size==1
         size--;
 
+        // REVIEW: we can move this to the first if case since it
+        //         can only occur when size==1
         if (size == 0) {
             firstIndex = 0;
         }

Propchange: harmony/enhanced/java/branches/java6/drlvm/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Aug 12 14:06:31 2010
@@ -1,5 +1,5 @@
 /harmony/enhanced/java/branches/mrh/drlvm:935751-941490
-/harmony/enhanced/java/trunk/drlvm:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731
+/harmony/enhanced/java/trunk/drlvm:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731,984779
 /harmony/enhanced/trunk/drlvm:476395-929252
 /harmony/enhanced/trunk/working_vm:476396-920147
 /incubator/harmony/enhanced/trunk/drlvm:292550-476394

Propchange: harmony/enhanced/java/branches/java6/jdktools/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Aug 12 14:06:31 2010
@@ -1,4 +1,4 @@
-/harmony/enhanced/java/trunk/jdktools:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731
+/harmony/enhanced/java/trunk/jdktools:929253-979569,979593,979613,979615,979647,979659,979682,979897,980326,980632,981356,981763,981811,981820,982146,982148,982183,982250,982377,982498,982614,982650,982777,982887,983879,983902,984367,984577,984597-984598,984613,984682,984708,984731,984779
 /harmony/enhanced/jdktools/trunk:630107-925933
 /harmony/enhanced/trunk/jdktools:476395-929252
 /harmony/enhanced/trunk/working_jdktools:476396-920147



Mime
View raw message