diff --git a/src/main/java/org/apache/commons/collections4/iterators/ArrayIterator.java b/src/main/java/org/apache/commons/collections4/iterators/ArrayIterator.java index 6e72e9db8..0bdd3506f 100644 --- a/src/main/java/org/apache/commons/collections4/iterators/ArrayIterator.java +++ b/src/main/java/org/apache/commons/collections4/iterators/ArrayIterator.java @@ -37,29 +37,17 @@ import org.apache.commons.collections4.ResettableIterator; */ public class ArrayIterator implements ResettableIterator { - // TODO Privatise fields? Mainly read-only access - /** The array to iterate over */ - protected Object array; + protected final Object array; /** The start index to loop from */ - protected int startIndex = 0; + protected final int startIndex; /** The end index to loop to */ - protected int endIndex = 0; + protected final int endIndex; /** The current iterator index */ protected int index = 0; // Constructors // ---------------------------------------------------------------------- - /** - * Constructor for use with setArray. - *

- * Using this constructor, the iterator is equivalent to an empty iterator - * until {@link #setArray(Object)} is called to establish the array to iterate over. - */ - public ArrayIterator() { - super(); - } - /** * Constructs an ArrayIterator that will iterate over the values in the * specified array. @@ -69,8 +57,7 @@ public class ArrayIterator implements ResettableIterator { * @throws NullPointerException if array is null */ public ArrayIterator(final Object array) { - super(); - setArray(array); + this(array, 0); } /** @@ -84,11 +71,7 @@ public class ArrayIterator implements ResettableIterator { * @throws IndexOutOfBoundsException if the index is invalid */ public ArrayIterator(final Object array, final int startIndex) { - super(); - setArray(array); - checkBound(startIndex, "start"); - this.startIndex = startIndex; - this.index = startIndex; + this(array, startIndex, Array.getLength(array)); } /** @@ -104,26 +87,30 @@ public class ArrayIterator implements ResettableIterator { */ public ArrayIterator(final Object array, final int startIndex, final int endIndex) { super(); - setArray(array); - checkBound(startIndex, "start"); - checkBound(endIndex, "end"); - if (endIndex < startIndex) { - throw new IllegalArgumentException("End index must not be less than start index."); - } + + this.array = array; this.startIndex = startIndex; this.endIndex = endIndex; this.index = startIndex; + + final int len = Array.getLength(array); + checkBound(startIndex, len, "start"); + checkBound(endIndex, len, "end"); + if (endIndex < startIndex) { + throw new IllegalArgumentException("End index must not be less than start index."); + } } /** * Checks whether the index is valid or not. * * @param bound the index to check + * @param len the length of the array * @param type the index type (for error messages) * @throws IndexOutOfBoundsException if the index is invalid */ - protected void checkBound(final int bound, final String type ) { - if (bound > this.endIndex) { + protected void checkBound(final int bound, final int len, final String type ) { + if (bound > len) { throw new ArrayIndexOutOfBoundsException( "Attempt to make an ArrayIterator that " + type + "s beyond the end of the array. " @@ -186,28 +173,23 @@ public class ArrayIterator implements ResettableIterator { } /** - * Sets the array that the ArrayIterator should iterate over. - *

- * If an array has previously been set (using the single-arg constructor - * or this method) then that array is discarded in favour of this one. - * Iteration is restarted at the start of the new array. - * Although this can be used to reset iteration, the {@link #reset()} method - * is a more effective choice. + * Gets the start index to loop from. * - * @param array the array that the iterator should iterate over. - * @throws IllegalArgumentException if array is not an array. - * @throws NullPointerException if array is null + * @return the start index + * @since 4.0 */ - public void setArray(final Object array) { - // Array.getLength throws IllegalArgumentException if the object is not - // an array or NullPointerException if the object is null. This call - // is made before saving the array and resetting the index so that the - // array iterator remains in a consistent state if the argument is not - // an array or is null. - this.endIndex = Array.getLength(array); - this.startIndex = 0; - this.array = array; - this.index = 0; + public int getStartIndex() { + return this.startIndex; + } + + /** + * Gets the end index to loop to. + * + * @return the end index + * @since 4.0 + */ + public int getEndIndex() { + return this.endIndex; } /** diff --git a/src/main/java/org/apache/commons/collections4/iterators/ArrayListIterator.java b/src/main/java/org/apache/commons/collections4/iterators/ArrayListIterator.java index 3505b1941..3f8b37795 100644 --- a/src/main/java/org/apache/commons/collections4/iterators/ArrayListIterator.java +++ b/src/main/java/org/apache/commons/collections4/iterators/ArrayListIterator.java @@ -54,16 +54,6 @@ public class ArrayListIterator extends ArrayIterator // Constructors // ---------------------------------------------------------------------- - /** - * Constructor for use with setArray. - *

- * Using this constructor, the iterator is equivalent to an empty iterator - * until {@link #setArray(Object)} is called to establish the array to iterate over. - */ - public ArrayListIterator() { - super(); - } - /** * Constructs an ArrayListIterator that will iterate over the values in the * specified array. @@ -88,7 +78,6 @@ public class ArrayListIterator extends ArrayIterator */ public ArrayListIterator(final Object array, final int startIndex) { super(array, startIndex); - this.startIndex = startIndex; } /** @@ -105,7 +94,6 @@ public class ArrayListIterator extends ArrayIterator */ public ArrayListIterator(final Object array, final int startIndex, final int endIndex) { super(array, startIndex, endIndex); - this.startIndex = startIndex; } // ListIterator interface diff --git a/src/main/java/org/apache/commons/collections4/iterators/ObjectArrayIterator.java b/src/main/java/org/apache/commons/collections4/iterators/ObjectArrayIterator.java index e0fd08a19..c11da69b5 100644 --- a/src/main/java/org/apache/commons/collections4/iterators/ObjectArrayIterator.java +++ b/src/main/java/org/apache/commons/collections4/iterators/ObjectArrayIterator.java @@ -36,27 +36,16 @@ import org.apache.commons.collections4.ResettableIterator; public class ObjectArrayIterator implements Iterator, ResettableIterator { - // TODO Privatise fields? Mainly read-only access - /** The array */ - protected E[] array = null; + protected final E[] array; /** The start index to loop from */ - protected int startIndex = 0; + protected final int startIndex; /** The end index to loop to */ - protected int endIndex = 0; + protected final int endIndex; /** The current iterator index */ protected int index = 0; - /** - * Constructor for use with setArray. - *

- * Using this constructor, the iterator is equivalent to an empty iterator - * until {@link #setArray} is called to establish the array to iterate over. - */ - public ObjectArrayIterator() { - super(); - } - + //------------------------------------------------------------------------- /** * Constructs an ObjectArrayIterator that will iterate over the values in the * specified array. @@ -153,36 +142,12 @@ public class ObjectArrayIterator /** * Gets the array that this iterator is iterating over. * - * @return the array this iterator iterates over, or null if - * the no-arg constructor was used and {@link #setArray} has never - * been called with a valid array. + * @return the array this iterator iterates over */ public E[] getArray() { return this.array; } - /** - * Sets the array that the ArrayIterator should iterate over. - *

- * This method may only be called once, otherwise an IllegalStateException - * will occur. - *

- * The {@link #reset} method can be used to reset the iterator if required. - * - * @param array the array that the iterator should iterate over - * @throws IllegalStateException if the array was set in the constructor - * @throws NullPointerException if array is null - */ - public void setArray(final E[] array) { - if (this.array != null) { - throw new IllegalStateException("The array to iterate over has already been set"); - } - this.array = array; - this.startIndex = 0; - this.endIndex = array.length; - this.index = 0; - } - /** * Gets the start index to loop from. * diff --git a/src/main/java/org/apache/commons/collections4/iterators/ObjectArrayListIterator.java b/src/main/java/org/apache/commons/collections4/iterators/ObjectArrayListIterator.java index 2028835e4..257bd8a1c 100644 --- a/src/main/java/org/apache/commons/collections4/iterators/ObjectArrayListIterator.java +++ b/src/main/java/org/apache/commons/collections4/iterators/ObjectArrayListIterator.java @@ -48,16 +48,7 @@ public class ObjectArrayListIterator extends ObjectArrayIterator */ private int lastItemIndex = -1; - /** - * Constructor for use with setArray. - *

- * Using this constructor, the iterator is equivalent to an empty iterator - * until {@link #setArray} is called to establish the array to iterate over. - */ - public ObjectArrayListIterator() { - super(); - } - + //------------------------------------------------------------------------- /** * Constructs an ObjectArrayListIterator that will iterate over the values in the * specified array. @@ -106,7 +97,7 @@ public class ObjectArrayListIterator extends ObjectArrayIterator * @return true if there is a previous element to return */ public boolean hasPrevious() { - return this.index > this.startIndex; + return this.index > getStartIndex(); } /** @@ -144,7 +135,7 @@ public class ObjectArrayListIterator extends ObjectArrayIterator * @return the index of the item to be retrieved next */ public int nextIndex() { - return this.index - this.startIndex; + return this.index - getStartIndex(); } /** @@ -153,7 +144,7 @@ public class ObjectArrayListIterator extends ObjectArrayIterator * @return the index of the item to be retrieved next */ public int previousIndex() { - return this.index - this.startIndex - 1; + return this.index - getStartIndex() - 1; } /** diff --git a/src/test/java/org/apache/commons/collections4/iterators/ArrayIterator2Test.java b/src/test/java/org/apache/commons/collections4/iterators/ArrayIterator2Test.java index 46fb8536a..8d313702b 100644 --- a/src/test/java/org/apache/commons/collections4/iterators/ArrayIterator2Test.java +++ b/src/test/java/org/apache/commons/collections4/iterators/ArrayIterator2Test.java @@ -81,29 +81,6 @@ public class ArrayIterator2Test extends AbstractIteratorTest { } } - // proves that an ArrayIterator set with the constructor has the same number of elements - // as an ArrayIterator set with setArray(Object) - public void testSetArray() { - final Iterator iter1 = makeArrayIterator(testArray); - int count1 = 0; - while (iter1.hasNext()) { - ++count1; - iter1.next(); - } - - assertEquals("the count should be right using the constructor", count1, testArray.length); - - final ArrayIterator iter2 = makeObject(); - iter2.setArray(testArray); - int count2 = 0; - while (iter2.hasNext()) { - ++count2; - iter2.next(); - } - - assertEquals("the count should be right using setArray(Object)", count2, testArray.length); - } - public void testIndexedArray() { Iterator iter = makeArrayIterator(testArray, 2); int count = 0; diff --git a/src/test/java/org/apache/commons/collections4/iterators/ArrayIteratorTest.java b/src/test/java/org/apache/commons/collections4/iterators/ArrayIteratorTest.java index d98b7adb1..b2704df5c 100644 --- a/src/test/java/org/apache/commons/collections4/iterators/ArrayIteratorTest.java +++ b/src/test/java/org/apache/commons/collections4/iterators/ArrayIteratorTest.java @@ -77,15 +77,6 @@ public class ArrayIteratorTest extends AbstractIteratorTest { } catch (final NullPointerException e) { // expected } - - final ArrayIterator iter = new ArrayIterator(); - try { - iter.setArray(null); - - fail("setArray(null) should throw a NullPointerException"); - } catch (final NullPointerException e) { - // expected - } } public void testReset() { diff --git a/src/test/java/org/apache/commons/collections4/iterators/ObjectArrayIteratorTest.java b/src/test/java/org/apache/commons/collections4/iterators/ObjectArrayIteratorTest.java index b3c6b9812..43ec17a38 100644 --- a/src/test/java/org/apache/commons/collections4/iterators/ObjectArrayIteratorTest.java +++ b/src/test/java/org/apache/commons/collections4/iterators/ObjectArrayIteratorTest.java @@ -94,26 +94,6 @@ public class ObjectArrayIteratorTest extends AbstractIteratorTest { } catch (final NullPointerException e) { // expected } - - final ObjectArrayIterator iter = makeArrayIterator(); - try { - iter.setArray(null); - - fail("setArray(null) should throw a NullPointerException"); - } catch (final NullPointerException e) { - // expected - } - } - - @SuppressWarnings("unchecked") - public void testDoubleSet() { - final ObjectArrayIterator it = makeArrayIterator(); - it.setArray((E[]) new String[0]); - try { - it.setArray((E[]) new String[0]); - fail(); - } catch (final IllegalStateException ex) { - } } @SuppressWarnings("unchecked")