RFR [8014066] Mistake in documentation of ArrayList#removeRange

[Martin Buchholz]

On Sun, Mar 16, 2014 at 3:37 PM, Ivan Gerasimov
<ivan.gerasimov at oracle.com>wrote:

> Here is yet another iteration of the fix:
> http://cr.openjdk.java.net/~igerasim/8014066/3/webrev/
>
> 1)
> The condition 'fromIndex >= size()' is removed from the spec.
> I prefer removing it rather than replacing it with 'fromIndex > size()'
> for two reasons:
> - 'fromIndex > size()' already follows on from two other conditions
> (toIndex > size() || toIndex < fromIndex);
> - it is consistent with the spec for CopyOnWriteArrayList#removeRange().
>
>
Here I'll agree with Ivan and disagree with David.


> 2)
> Kept the check for 'fromIndex > toIndex' in removeRange().
> While I understand that this should not add anything significant to the
> current code, as currently removeRange() is always called with valid
> arguments.
> However, if it is stated in the spec that in case of 'fromIndex > toIndex'
> an exception is thrown, I believe it should be thrown, otherwise why it's
> stated?
>
>
It's a good point.  ArrayList is performance critical, but removeRange of
course far less so.
Nevertheless, when adding range checking code like this, create a helper
method to create the exception message as is already done in ArrayList:

    /**
     * Constructs an IndexOutOfBoundsException detail message.
     * Of the many possible refactorings of the error handling code,
     * this "outlining" performs best with both server and client VMs.
     */
    private String outOfBoundsMsg(int index) {
        return "Index: "+index+", Size: "+size;
    }



> 3)
> Moved the test to MOAT.java
> The test looks a bit foreign over there, but reuses some of the
> infrastructure.
>

The idea of MOAT is to do NxM testing (many implementations, many
assertions).  When testing only one implementation, don't put it in MOAT.

Today a test like MOAT is looking quite dated.  Today we would make use of
lambda and testng instead of my old homebrew infrastructure.

Here's some test code for you to add to MOAT (please incorporate):

 diff --git a/test/java/util/Collection/MOAT.java
b/test/java/util/Collection/MOAT.java
--- a/test/java/util/Collection/MOAT.java
+++ b/test/java/util/Collection/MOAT.java
@@ -54,6 +54,7 @@
 import java.util.*;
 import java.util.concurrent.*;
 import static java.util.Collections.*;
+import java.lang.reflect.*;

 public class MOAT {
     public static void realMain(String[] args) {
@@ -721,6 +722,28 @@

         equal(l instanceof RandomAccess,
               l.subList(0,0) instanceof RandomAccess);
+
+        l.iterator();
+        l.listIterator();
+        l.listIterator(0);
+        l.listIterator(l.size());
+        THROWS(IndexOutOfBoundsException.class,
+               new Fun(){void f(){l.listIterator(-1);}},
+               new Fun(){void f(){l.listIterator(l.size() + 1);}});
+
+        if (l instanceof AbstractList) {
+            try {
+                int size = l.size();
+                AbstractList<Integer> abList = (AbstractList<Integer>) l;
+                Method m =
AbstractList.class.getDeclaredMethod("removeRange", new Class[] {
int.class, int.class });
+                m.setAccessible(true);
+                m.invoke(abList, new Object[] { 0, 0 });
+                m.invoke(abList, new Object[] { size, size });
+                equal(size, l.size());
+            }
+            catch (UnsupportedOperationException ignored) {/* OK */}
+            catch (Throwable t) { unexpected(t); }
+        }
     }

     private static void testCollection(Collection<Integer> c) {