You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pivot.apache.org by no...@apache.org on 2009/11/13 14:15:02 UTC
svn commit: r835842 - in
/incubator/pivot/trunk/core/src/org/apache/pivot/collections: ./ concurrent/
Author: noelgrandin
Date: Fri Nov 13 13:15:02 2009
New Revision: 835842
URL: http://svn.apache.org/viewvc?rev=835842&view=rev
Log:
make the exception throwing more descriptive
Added:
incubator/pivot/trunk/core/src/org/apache/pivot/collections/CollectionArgChecks.java
Modified:
incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayList.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayQueue.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayStack.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/Dictionary.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumList.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumMap.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedList.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedQueue.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedStack.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedList.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedMap.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedQueue.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedSet.java
incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedStack.java
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayList.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayList.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayList.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayList.java Fri Nov 13 13:15:02 2009
@@ -91,10 +91,7 @@
@Override
public void insert(T item) {
- if (index < 0
- || index >= ArrayList.this.length) {
- throw new IllegalStateException();
- }
+ indexBoundsCheck();
ArrayList.this.insert(item, index);
modificationCount++;
@@ -102,10 +99,7 @@
@Override
public void update(T item) {
- if (index < 0
- || index >= ArrayList.this.length) {
- throw new IllegalStateException();
- }
+ indexBoundsCheck();
ArrayList.this.update(index, item);
modificationCount++;
@@ -113,14 +107,17 @@
@Override
public void remove() {
- if (index < 0
- || index >= ArrayList.this.length) {
- throw new IllegalStateException();
- }
+ indexBoundsCheck();
ArrayList.this.remove(index, 1);
modificationCount++;
}
+
+ private void indexBoundsCheck() {
+ if (index < 0 || index >+ ArrayList.this.length) {
+ throw new IllegalStateException("index " + index + " out of bounds");
+ }
+ }
}
@@ -144,9 +141,7 @@
}
public ArrayList(int capacity) {
- if (capacity < 0) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.zeroOrGreater("capacity", capacity);
items = new Object[capacity];
}
@@ -156,18 +151,8 @@
}
public ArrayList(T[] items, int index, int count) {
- if (items == null) {
- throw new IllegalArgumentException();
- }
-
- if (count < 0) {
- throw new IllegalArgumentException();
- }
-
- if (index < 0
- || index + count > items.length) {
- throw new IndexOutOfBoundsException();
- }
+ CollectionArgChecks.notNull("items", items);
+ CollectionArgChecks.indexBounds(index, count, 0, items.length);
this.items = new Object[count];
System.arraycopy(items, index, this.items, 0, count);
@@ -180,18 +165,8 @@
}
public ArrayList(Sequence<T> items, int index, int count) {
- if (items == null) {
- throw new IllegalArgumentException();
- }
-
- if (count < 0) {
- throw new IllegalArgumentException();
- }
-
- if (index < 0
- || index + count > items.getLength()) {
- throw new IndexOutOfBoundsException();
- }
+ CollectionArgChecks.notNull("items", items);
+ CollectionArgChecks.indexBounds(index, count, 0, items.getLength());
this.items = new Object[count];
@@ -207,18 +182,8 @@
}
public ArrayList(ArrayList<T> arrayList, int index, int count) {
- if (arrayList == null) {
- throw new IllegalArgumentException();
- }
-
- if (count < 0) {
- throw new IllegalArgumentException();
- }
-
- if (index < 0
- || index + count > arrayList.length) {
- throw new IndexOutOfBoundsException();
- }
+ CollectionArgChecks.notNull("arrayList", arrayList);
+ CollectionArgChecks.indexBounds(index, count, 0, arrayList.length);
items = new Object[count];
length = count;
@@ -255,10 +220,7 @@
}
private void insert(T item, int index, boolean validate) {
- if (index < 0
- || index > length) {
- throw new IndexOutOfBoundsException();
- }
+ CollectionArgChecks.indexBounds(index, 0, length);
if (comparator != null
&& validate) {
@@ -288,10 +250,7 @@
@SuppressWarnings("unchecked")
@Override
public T update(int index, T item) {
- if (index < 0
- || index >= length) {
- throw new IndexOutOfBoundsException();
- }
+ CollectionArgChecks.indexBounds(index, 0, length - 1);
T previousItem = (T)items[index];
@@ -336,10 +295,7 @@
@SuppressWarnings("unchecked")
@Override
public Sequence<T> remove(int index, int count) {
- if (index < 0
- || index + count > length) {
- throw new IndexOutOfBoundsException();
- }
+ CollectionArgChecks.indexBounds(index, count, 0, length);
ArrayList<T> removed = new ArrayList<T>((T[])items, index, count);
@@ -380,10 +336,7 @@
@SuppressWarnings("unchecked")
@Override
public T get(int index) {
- if (index < 0
- || index >= length) {
- throw new IndexOutOfBoundsException();
- }
+ CollectionArgChecks.indexBounds(index, 0, length);
return (T)items[index];
}
@@ -551,10 +504,8 @@
@SuppressWarnings("unchecked")
public static <T> void sort(ArrayList<T> arrayList, int from, int to, Comparator<T> comparator) {
- if (arrayList == null
- || comparator == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("arrayList", arrayList);
+ CollectionArgChecks.notNull("comparator", comparator);
Arrays.sort((T[])arrayList.items, from, to, comparator);
@@ -572,11 +523,9 @@
@SuppressWarnings("unchecked")
public static <T> int binarySearch(ArrayList<T> arrayList, T item, Comparator<T> comparator) {
- if (arrayList == null
- || item == null
- || comparator == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("arrayList", arrayList);
+ CollectionArgChecks.notNull("comparator", comparator);
+ CollectionArgChecks.notNull("item", item);
int index = Arrays.binarySearch((T[])arrayList.items, 0, arrayList.length, item, comparator);
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayQueue.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayQueue.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayQueue.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayQueue.java Fri Nov 13 13:15:02 2009
@@ -61,7 +61,7 @@
public T dequeue() {
int length = arrayList.getLength();
if (length == 0) {
- throw new IllegalStateException();
+ throw new IllegalStateException("queue is empty");
}
T item = arrayList.remove(length - 1, 1).get(0);
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayStack.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayStack.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayStack.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayStack.java Fri Nov 13 13:15:02 2009
@@ -56,7 +56,7 @@
public T pop() {
int length = arrayList.getLength();
if (length == 0) {
- throw new IllegalStateException();
+ throw new IllegalStateException("queue is empty");
}
T item = arrayList.remove(length - 1, 1).get(0);
Added: incubator/pivot/trunk/core/src/org/apache/pivot/collections/CollectionArgChecks.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/CollectionArgChecks.java?rev=835842&view=auto
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/CollectionArgChecks.java (added)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/CollectionArgChecks.java Fri Nov 13 13:15:02 2009
@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License,
+ * Version 2.0 (the "License"); you may not use this file except in
+ * compliance with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.pivot.collections;
+
+/**
+ * Implements various assert-style checking for the Pivot collections classes.
+ * Throws nice descriptive exceptions if something goes wrong.
+ *
+ * @author Noel Grandin
+ */
+public class CollectionArgChecks {
+
+ public static void notNull(String fieldName, Object field) {
+ if (field == null) {
+ throw new IllegalArgumentException(fieldName + " cannot be null");
+ }
+ }
+
+ public static void zeroOrGreater(String fieldName, int field) {
+ if (field < 0) {
+ throw new IllegalArgumentException(fieldName + " " + field + " cannot be < 0");
+ }
+ }
+
+ public static void indexBounds(int index, int boundStart, int boundEnd) {
+ if (index < boundStart || index > boundEnd) {
+ throw new IndexOutOfBoundsException("index " + index + " out of bounds");
+ }
+ }
+
+ public static void indexBounds(int index, int count, int boundStart, int boundEnd) {
+ if (count < 0) {
+ throw new IllegalArgumentException();
+ }
+ if (index < boundStart) {
+ throw new IndexOutOfBoundsException("index " + index + " out of bounds");
+ }
+ if (index + count > boundEnd) {
+ throw new IndexOutOfBoundsException("index + count " + index + "," + count + " out of range");
+ }
+ }
+}
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/Dictionary.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/Dictionary.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/Dictionary.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/Dictionary.java Fri Nov 13 13:15:02 2009
@@ -28,9 +28,7 @@
public final V value;
public Pair(K key, V value) {
- if (key == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("key", key);
this.key = key;
this.value = value;
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumList.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumList.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumList.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumList.java Fri Nov 13 13:15:02 2009
@@ -108,9 +108,7 @@
@Override
public int indexOf(E item) {
- if (item == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("item", item);
return item.ordinal();
}
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumMap.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumMap.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumMap.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumMap.java Fri Nov 13 13:15:02 2009
@@ -45,9 +45,7 @@
@SuppressWarnings("unchecked")
@Override
public V get(E key) {
- if (key == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("key", key);
return (V)values[key.ordinal()];
}
@@ -55,9 +53,7 @@
@SuppressWarnings("unchecked")
@Override
public V put(E key, V value) {
- if (key == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("key", key);
int ordinal = key.ordinal();
V previousValue = (V)values[ordinal];
@@ -76,9 +72,7 @@
@SuppressWarnings("unchecked")
@Override
public V remove(E key) {
- if (key == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("key", key);
V value = null;
if (keySet.contains(key)) {
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedList.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedList.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedList.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedList.java Fri Nov 13 13:15:02 2009
@@ -305,10 +305,7 @@
@Override
public void insert(T item, int index) {
- if (index < 0
- || index > length) {
- throw new IndexOutOfBoundsException();
- }
+ CollectionArgChecks.indexBounds(index, 0, length);
Node<T> next = null;
Node<T> previous = null;
@@ -348,10 +345,7 @@
@Override
public T update(int index, T item) {
- if (index < 0
- || index >= length) {
- throw new IndexOutOfBoundsException();
- }
+ CollectionArgChecks.indexBounds(index, 0, length - 1);
// Get the previous item at index
Node<T> node = getNode(index);
@@ -409,10 +403,7 @@
@Override
public Sequence<T> remove(int index, int count) {
- if (index < 0
- || index + count > length) {
- throw new IndexOutOfBoundsException();
- }
+ CollectionArgChecks.indexBounds(index, count, 0, length);
LinkedList<T> removed = new LinkedList<T>();
@@ -479,10 +470,7 @@
@Override
public T get(int index) {
- if (index < 0
- || index >= length) {
- throw new IndexOutOfBoundsException();
- }
+ CollectionArgChecks.indexBounds(index, 0, length - 1);
Node<T> node = getNode(index);
return node.item;
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedQueue.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedQueue.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedQueue.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedQueue.java Fri Nov 13 13:15:02 2009
@@ -57,7 +57,7 @@
public T dequeue() {
int length = linkedList.getLength();
if (length == 0) {
- throw new IllegalStateException();
+ throw new IllegalStateException("queue is empty");
}
T item = linkedList.remove(length - 1, 1).get(0);
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedStack.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedStack.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedStack.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedStack.java Fri Nov 13 13:15:02 2009
@@ -51,7 +51,7 @@
public T pop() {
int length = linkedList.getLength();
if (length == 0) {
- throw new IllegalStateException();
+ throw new IllegalStateException("queue is empty");
}
T item = linkedList.remove(length - 1, 1).get(0);
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedList.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedList.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedList.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedList.java Fri Nov 13 13:15:02 2009
@@ -19,6 +19,7 @@
import java.util.Comparator;
import java.util.Iterator;
+import org.apache.pivot.collections.CollectionArgChecks;
import org.apache.pivot.collections.List;
import org.apache.pivot.collections.ListListener;
import org.apache.pivot.collections.Sequence;
@@ -72,9 +73,7 @@
private SynchronizedListListenerList<T> listListeners = new SynchronizedListListenerList<T>();
public SynchronizedList(List<T> list) {
- if (list == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("list", list);
this.list = list;
}
@@ -107,7 +106,7 @@
public synchronized int remove (T item) {
int index = indexOf(item);
if (index == -1) {
- throw new IllegalArgumentException();
+ throw new IllegalArgumentException("item not in list");
}
remove(index, 1);
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedMap.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedMap.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedMap.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedMap.java Fri Nov 13 13:15:02 2009
@@ -19,6 +19,7 @@
import java.util.Comparator;
import java.util.Iterator;
+import org.apache.pivot.collections.CollectionArgChecks;
import org.apache.pivot.collections.Map;
import org.apache.pivot.collections.MapListener;
import org.apache.pivot.util.ImmutableIterator;
@@ -70,9 +71,7 @@
private SynchronizedMapListenerList<K, V> mapListeners = new SynchronizedMapListenerList<K, V>();
public SynchronizedMap(Map<K, V> map) {
- if (map == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("map", map);
this.map = map;
}
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedQueue.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedQueue.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedQueue.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedQueue.java Fri Nov 13 13:15:02 2009
@@ -19,6 +19,7 @@
import java.util.Comparator;
import java.util.Iterator;
+import org.apache.pivot.collections.CollectionArgChecks;
import org.apache.pivot.collections.Queue;
import org.apache.pivot.collections.QueueListener;
import org.apache.pivot.util.ImmutableIterator;
@@ -56,9 +57,7 @@
private SynchronizedQueueListenerList<T> queueListeners = new SynchronizedQueueListenerList<T>();
public SynchronizedQueue(Queue<T> queue) {
- if (queue == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("queue", queue);
this.queue = queue;
}
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedSet.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedSet.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedSet.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedSet.java Fri Nov 13 13:15:02 2009
@@ -19,6 +19,7 @@
import java.util.Comparator;
import java.util.Iterator;
+import org.apache.pivot.collections.CollectionArgChecks;
import org.apache.pivot.collections.Set;
import org.apache.pivot.collections.SetListener;
import org.apache.pivot.util.ImmutableIterator;
@@ -65,9 +66,7 @@
private SynchronizedSetListenerList<E> setListeners = new SynchronizedSetListenerList<E>();
public SynchronizedSet(Set<E> set) {
- if (set == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("set", set);
this.set = set;
}
Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedStack.java
URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedStack.java?rev=835842&r1=835841&r2=835842&view=diff
==============================================================================
--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedStack.java (original)
+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedStack.java Fri Nov 13 13:15:02 2009
@@ -19,6 +19,7 @@
import java.util.Comparator;
import java.util.Iterator;
+import org.apache.pivot.collections.CollectionArgChecks;
import org.apache.pivot.collections.Stack;
import org.apache.pivot.collections.StackListener;
import org.apache.pivot.util.ImmutableIterator;
@@ -56,9 +57,7 @@
private SynchronizedStackListenerList<T> stackListeners = new SynchronizedStackListenerList<T>();
public SynchronizedStack(Stack<T> stack) {
- if (stack == null) {
- throw new IllegalArgumentException();
- }
+ CollectionArgChecks.notNull("stack", stack);
this.stack = stack;
}
Re: svn commit: r835842 - in /incubator/pivot/trunk/core/src/org/apache/pivot/collections:
./ concurrent/
Posted by Sandro Martini <sa...@gmail.com>.
Ok, I understand your reasons.
Bye
Re: svn commit: r835842 - in /incubator/pivot/trunk/core/src/org/apache/pivot/collections:
./ concurrent/
Posted by Noel Grandin <no...@gmail.com>.
Hi
Those have less value for me. I implemented the checks for 3 reasons.
(1) to disambiguate the cases where we throw multiple exceptions in the
same spot for different arguments
(2) to display the index in IndexOutOfBoundsException, because knowing
what the index value was is often enough to pin-point a bug.
(3) collections code get used __a lot__ so I see more exceptions coming
from there.
The stuff in wtk/ is less interesting to me because it doesn't match any
of these criteria.
Generally the exception throwing sites are single-case, or they're in
weird out of the way places where exceptions are not likely to happen much.
-- Noel.
Sandro Martini wrote:
> Hi Noel,
> great job.
>
> What do you think on add also a method like
>
> public static void notEmpty(String fieldName, Object field) {
>
> in CollectionArgChecks ?
>
>
> And for many WTK Checks, a notNegative(...) and maybe another handling
> also the -1 case ?
>
>
> Comments ?
>
> Bye
>
Re: svn commit: r835842 - in /incubator/pivot/trunk/core/src/org/apache/pivot/collections:
./ concurrent/
Posted by Sandro Martini <sa...@gmail.com>.
Hi Noel,
great job.
What do you think on add also a method like
public static void notEmpty(String fieldName, Object field) {
in CollectionArgChecks ?
And for many WTK Checks, a notNegative(...) and maybe another handling
also the -1 case ?
Comments ?
Bye
Re: svn commit: r835842 - in
/incubator/pivot/trunk/core/src/org/apache/pivot/collections: ./ concurrent/
Posted by Greg Brown <gk...@mac.com>.
Agreed.
On Nov 16, 2009, at 1:32 PM, Todd Volkert wrote:
> Actually, I think it's probably best to remove the class and move
> verifyNotNull() into the constituent classes inline. We have a ton
> of null
> arg checks elsewhere in the framework that don't use a dedicated arg
> check
> class, so it feels weird to introduce one here. I'm open to
> discussing the
> existence of such classes (though I personally tend to shy away from
> them),
> but using it in this one package seems inconsistent and out of place.
>
> -T
>
> On Sat, Nov 14, 2009 at 12:47 AM, Noel Grandin
> <no...@gmail.com>wrote:
>
>> I've renamed the methods and moved most of them into the caller
>> classes.
>>
>> It seems a shame to move verifyNotNull(), though, because it is used
>> in so many places ... ?
>>
>> I can't make it package-private because it is used in
>> collections..concurrent
>>
>>
>> On Fri, Nov 13, 2009 at 20:11, Todd Volkert <tv...@gmail.com>
>> wrote:
>>>> - I would prefer to see the methods named "validateXXX()" or
>> "verifyXXX()".
>>>>
>>>
>>> +1 - I like these names better.
>>>
>>>
>>>>
>>>> - I would prefer to see them defined within the classes that call
>>>> them,
>>>> rather than delegating to a separate CollectionArgChecks class.
>>>> Even if
>> it
>>>> introduces a bit of redundancy, I think the encapsulation offered
>>>> by
>> this
>>>> approach is preferable.
>>>>
>>>
>>> I personally don't actually mind the separate class in this case,
>>> but I
>>> think it should be package private.
>>>
>>
Re: svn commit: r835842 - in
/incubator/pivot/trunk/core/src/org/apache/pivot/collections: ./ concurrent/
Posted by Greg Brown <gk...@me.com>.
Agreed.
On Nov 16, 2009, at 1:32 PM, Todd Volkert wrote:
> Actually, I think it's probably best to remove the class and move
> verifyNotNull() into the constituent classes inline. We have a ton
> of null
> arg checks elsewhere in the framework that don't use a dedicated arg
> check
> class, so it feels weird to introduce one here. I'm open to
> discussing the
> existence of such classes (though I personally tend to shy away from
> them),
> but using it in this one package seems inconsistent and out of place.
>
> -T
>
> On Sat, Nov 14, 2009 at 12:47 AM, Noel Grandin
> <no...@gmail.com>wrote:
>
>> I've renamed the methods and moved most of them into the caller
>> classes.
>>
>> It seems a shame to move verifyNotNull(), though, because it is used
>> in so many places ... ?
>>
>> I can't make it package-private because it is used in
>> collections..concurrent
>>
>>
>> On Fri, Nov 13, 2009 at 20:11, Todd Volkert <tv...@gmail.com>
>> wrote:
>>>> - I would prefer to see the methods named "validateXXX()" or
>> "verifyXXX()".
>>>>
>>>
>>> +1 - I like these names better.
>>>
>>>
>>>>
>>>> - I would prefer to see them defined within the classes that call
>>>> them,
>>>> rather than delegating to a separate CollectionArgChecks class.
>>>> Even if
>> it
>>>> introduces a bit of redundancy, I think the encapsulation offered
>>>> by
>> this
>>>> approach is preferable.
>>>>
>>>
>>> I personally don't actually mind the separate class in this case,
>>> but I
>>> think it should be package private.
>>>
>>
Re: svn commit: r835842 - in /incubator/pivot/trunk/core/src/org/apache/pivot/collections:
./ concurrent/
Posted by Todd Volkert <tv...@gmail.com>.
Actually, I think it's probably best to remove the class and move
verifyNotNull() into the constituent classes inline. We have a ton of null
arg checks elsewhere in the framework that don't use a dedicated arg check
class, so it feels weird to introduce one here. I'm open to discussing the
existence of such classes (though I personally tend to shy away from them),
but using it in this one package seems inconsistent and out of place.
-T
On Sat, Nov 14, 2009 at 12:47 AM, Noel Grandin <no...@gmail.com>wrote:
> I've renamed the methods and moved most of them into the caller classes.
>
> It seems a shame to move verifyNotNull(), though, because it is used
> in so many places ... ?
>
> I can't make it package-private because it is used in
> collections..concurrent
>
>
> On Fri, Nov 13, 2009 at 20:11, Todd Volkert <tv...@gmail.com> wrote:
> >> - I would prefer to see the methods named "validateXXX()" or
> "verifyXXX()".
> >>
> >
> > +1 - I like these names better.
> >
> >
> >>
> >> - I would prefer to see them defined within the classes that call them,
> >> rather than delegating to a separate CollectionArgChecks class. Even if
> it
> >> introduces a bit of redundancy, I think the encapsulation offered by
> this
> >> approach is preferable.
> >>
> >
> > I personally don't actually mind the separate class in this case, but I
> > think it should be package private.
> >
>
Re: svn commit: r835842 - in /incubator/pivot/trunk/core/src/org/apache/pivot/collections:
./ concurrent/
Posted by Noel Grandin <no...@gmail.com>.
I've renamed the methods and moved most of them into the caller classes.
It seems a shame to move verifyNotNull(), though, because it is used
in so many places ... ?
I can't make it package-private because it is used in collections..concurrent
On Fri, Nov 13, 2009 at 20:11, Todd Volkert <tv...@gmail.com> wrote:
>> - I would prefer to see the methods named "validateXXX()" or "verifyXXX()".
>>
>
> +1 - I like these names better.
>
>
>>
>> - I would prefer to see them defined within the classes that call them,
>> rather than delegating to a separate CollectionArgChecks class. Even if it
>> introduces a bit of redundancy, I think the encapsulation offered by this
>> approach is preferable.
>>
>
> I personally don't actually mind the separate class in this case, but I
> think it should be package private.
>
Re: svn commit: r835842 - in /incubator/pivot/trunk/core/src/org/apache/pivot/collections:
./ concurrent/
Posted by Todd Volkert <tv...@gmail.com>.
> - I would prefer to see the methods named "validateXXX()" or "verifyXXX()".
>
+1 - I like these names better.
>
> - I would prefer to see them defined within the classes that call them,
> rather than delegating to a separate CollectionArgChecks class. Even if it
> introduces a bit of redundancy, I think the encapsulation offered by this
> approach is preferable.
>
I personally don't actually mind the separate class in this case, but I
think it should be package private.
Re: svn commit: r835842 - in
/incubator/pivot/trunk/core/src/org/apache/pivot/collections: ./ concurrent/
Posted by Greg Brown <gk...@mac.com>.
I see the value in this change, but I am not crazy about the class or method names:
- I would prefer to see the methods named "validateXXX()" or "verifyXXX()".
- I would prefer to see them defined within the classes that call them, rather than delegating to a separate CollectionArgChecks class. Even if it introduces a bit of redundancy, I think the encapsulation offered by this approach is preferable.
G
On Friday, November 13, 2009, at 08:15AM, <no...@apache.org> wrote:
>Author: noelgrandin
>Date: Fri Nov 13 13:15:02 2009
>New Revision: 835842
>
>URL: http://svn.apache.org/viewvc?rev=835842&view=rev
>Log:
>make the exception throwing more descriptive
>
>Added:
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/CollectionArgChecks.java
>Modified:
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayList.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayQueue.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayStack.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/Dictionary.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumList.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumMap.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedList.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedQueue.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedStack.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedList.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedMap.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedQueue.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedSet.java
> incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedStack.java
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayList.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayList.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayList.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayList.java Fri Nov 13 13:15:02 2009
>@@ -91,10 +91,7 @@
>
> @Override
> public void insert(T item) {
>- if (index < 0
>- || index >= ArrayList.this.length) {
>- throw new IllegalStateException();
>- }
>+ indexBoundsCheck();
>
> ArrayList.this.insert(item, index);
> modificationCount++;
>@@ -102,10 +99,7 @@
>
> @Override
> public void update(T item) {
>- if (index < 0
>- || index >= ArrayList.this.length) {
>- throw new IllegalStateException();
>- }
>+ indexBoundsCheck();
>
> ArrayList.this.update(index, item);
> modificationCount++;
>@@ -113,14 +107,17 @@
>
> @Override
> public void remove() {
>- if (index < 0
>- || index >= ArrayList.this.length) {
>- throw new IllegalStateException();
>- }
>+ indexBoundsCheck();
>
> ArrayList.this.remove(index, 1);
> modificationCount++;
> }
>+
>+ private void indexBoundsCheck() {
>+ if (index < 0 || index >+ ArrayList.this.length) {
>+ throw new IllegalStateException("index " + index + " out of bounds");
>+ }
>+ }
> }
>
>
>@@ -144,9 +141,7 @@
> }
>
> public ArrayList(int capacity) {
>- if (capacity < 0) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.zeroOrGreater("capacity", capacity);
>
> items = new Object[capacity];
> }
>@@ -156,18 +151,8 @@
> }
>
> public ArrayList(T[] items, int index, int count) {
>- if (items == null) {
>- throw new IllegalArgumentException();
>- }
>-
>- if (count < 0) {
>- throw new IllegalArgumentException();
>- }
>-
>- if (index < 0
>- || index + count > items.length) {
>- throw new IndexOutOfBoundsException();
>- }
>+ CollectionArgChecks.notNull("items", items);
>+ CollectionArgChecks.indexBounds(index, count, 0, items.length);
>
> this.items = new Object[count];
> System.arraycopy(items, index, this.items, 0, count);
>@@ -180,18 +165,8 @@
> }
>
> public ArrayList(Sequence<T> items, int index, int count) {
>- if (items == null) {
>- throw new IllegalArgumentException();
>- }
>-
>- if (count < 0) {
>- throw new IllegalArgumentException();
>- }
>-
>- if (index < 0
>- || index + count > items.getLength()) {
>- throw new IndexOutOfBoundsException();
>- }
>+ CollectionArgChecks.notNull("items", items);
>+ CollectionArgChecks.indexBounds(index, count, 0, items.getLength());
>
> this.items = new Object[count];
>
>@@ -207,18 +182,8 @@
> }
>
> public ArrayList(ArrayList<T> arrayList, int index, int count) {
>- if (arrayList == null) {
>- throw new IllegalArgumentException();
>- }
>-
>- if (count < 0) {
>- throw new IllegalArgumentException();
>- }
>-
>- if (index < 0
>- || index + count > arrayList.length) {
>- throw new IndexOutOfBoundsException();
>- }
>+ CollectionArgChecks.notNull("arrayList", arrayList);
>+ CollectionArgChecks.indexBounds(index, count, 0, arrayList.length);
>
> items = new Object[count];
> length = count;
>@@ -255,10 +220,7 @@
> }
>
> private void insert(T item, int index, boolean validate) {
>- if (index < 0
>- || index > length) {
>- throw new IndexOutOfBoundsException();
>- }
>+ CollectionArgChecks.indexBounds(index, 0, length);
>
> if (comparator != null
> && validate) {
>@@ -288,10 +250,7 @@
> @SuppressWarnings("unchecked")
> @Override
> public T update(int index, T item) {
>- if (index < 0
>- || index >= length) {
>- throw new IndexOutOfBoundsException();
>- }
>+ CollectionArgChecks.indexBounds(index, 0, length - 1);
>
> T previousItem = (T)items[index];
>
>@@ -336,10 +295,7 @@
> @SuppressWarnings("unchecked")
> @Override
> public Sequence<T> remove(int index, int count) {
>- if (index < 0
>- || index + count > length) {
>- throw new IndexOutOfBoundsException();
>- }
>+ CollectionArgChecks.indexBounds(index, count, 0, length);
>
> ArrayList<T> removed = new ArrayList<T>((T[])items, index, count);
>
>@@ -380,10 +336,7 @@
> @SuppressWarnings("unchecked")
> @Override
> public T get(int index) {
>- if (index < 0
>- || index >= length) {
>- throw new IndexOutOfBoundsException();
>- }
>+ CollectionArgChecks.indexBounds(index, 0, length);
>
> return (T)items[index];
> }
>@@ -551,10 +504,8 @@
>
> @SuppressWarnings("unchecked")
> public static <T> void sort(ArrayList<T> arrayList, int from, int to, Comparator<T> comparator) {
>- if (arrayList == null
>- || comparator == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("arrayList", arrayList);
>+ CollectionArgChecks.notNull("comparator", comparator);
>
> Arrays.sort((T[])arrayList.items, from, to, comparator);
>
>@@ -572,11 +523,9 @@
>
> @SuppressWarnings("unchecked")
> public static <T> int binarySearch(ArrayList<T> arrayList, T item, Comparator<T> comparator) {
>- if (arrayList == null
>- || item == null
>- || comparator == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("arrayList", arrayList);
>+ CollectionArgChecks.notNull("comparator", comparator);
>+ CollectionArgChecks.notNull("item", item);
>
> int index = Arrays.binarySearch((T[])arrayList.items, 0, arrayList.length, item, comparator);
>
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayQueue.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayQueue.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayQueue.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayQueue.java Fri Nov 13 13:15:02 2009
>@@ -61,7 +61,7 @@
> public T dequeue() {
> int length = arrayList.getLength();
> if (length == 0) {
>- throw new IllegalStateException();
>+ throw new IllegalStateException("queue is empty");
> }
>
> T item = arrayList.remove(length - 1, 1).get(0);
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayStack.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayStack.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayStack.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/ArrayStack.java Fri Nov 13 13:15:02 2009
>@@ -56,7 +56,7 @@
> public T pop() {
> int length = arrayList.getLength();
> if (length == 0) {
>- throw new IllegalStateException();
>+ throw new IllegalStateException("queue is empty");
> }
>
> T item = arrayList.remove(length - 1, 1).get(0);
>
>Added: incubator/pivot/trunk/core/src/org/apache/pivot/collections/CollectionArgChecks.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/CollectionArgChecks.java?rev=835842&view=auto
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/CollectionArgChecks.java (added)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/CollectionArgChecks.java Fri Nov 13 13:15:02 2009
>@@ -0,0 +1,56 @@
>+/*
>+ * Licensed to the Apache Software Foundation (ASF) under one or more
>+ * contributor license agreements. See the NOTICE file distributed with
>+ * this work for additional information regarding copyright ownership.
>+ * The ASF licenses this file to you under the Apache License,
>+ * Version 2.0 (the "License"); you may not use this file except in
>+ * compliance with the License. You may obtain a copy of the License at
>+ *
>+ * http://www.apache.org/licenses/LICENSE-2.0
>+ *
>+ * Unless required by applicable law or agreed to in writing, software
>+ * distributed under the License is distributed on an "AS IS" BASIS,
>+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>+ * See the License for the specific language governing permissions and
>+ * limitations under the License.
>+ */
>+package org.apache.pivot.collections;
>+
>+/**
>+ * Implements various assert-style checking for the Pivot collections classes.
>+ * Throws nice descriptive exceptions if something goes wrong.
>+ *
>+ * @author Noel Grandin
>+ */
>+public class CollectionArgChecks {
>+
>+ public static void notNull(String fieldName, Object field) {
>+ if (field == null) {
>+ throw new IllegalArgumentException(fieldName + " cannot be null");
>+ }
>+ }
>+
>+ public static void zeroOrGreater(String fieldName, int field) {
>+ if (field < 0) {
>+ throw new IllegalArgumentException(fieldName + " " + field + " cannot be < 0");
>+ }
>+ }
>+
>+ public static void indexBounds(int index, int boundStart, int boundEnd) {
>+ if (index < boundStart || index > boundEnd) {
>+ throw new IndexOutOfBoundsException("index " + index + " out of bounds");
>+ }
>+ }
>+
>+ public static void indexBounds(int index, int count, int boundStart, int boundEnd) {
>+ if (count < 0) {
>+ throw new IllegalArgumentException();
>+ }
>+ if (index < boundStart) {
>+ throw new IndexOutOfBoundsException("index " + index + " out of bounds");
>+ }
>+ if (index + count > boundEnd) {
>+ throw new IndexOutOfBoundsException("index + count " + index + "," + count + " out of range");
>+ }
>+ }
>+}
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/Dictionary.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/Dictionary.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/Dictionary.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/Dictionary.java Fri Nov 13 13:15:02 2009
>@@ -28,9 +28,7 @@
> public final V value;
>
> public Pair(K key, V value) {
>- if (key == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("key", key);
>
> this.key = key;
> this.value = value;
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumList.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumList.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumList.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumList.java Fri Nov 13 13:15:02 2009
>@@ -108,9 +108,7 @@
>
> @Override
> public int indexOf(E item) {
>- if (item == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("item", item);
>
> return item.ordinal();
> }
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumMap.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumMap.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumMap.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/EnumMap.java Fri Nov 13 13:15:02 2009
>@@ -45,9 +45,7 @@
> @SuppressWarnings("unchecked")
> @Override
> public V get(E key) {
>- if (key == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("key", key);
>
> return (V)values[key.ordinal()];
> }
>@@ -55,9 +53,7 @@
> @SuppressWarnings("unchecked")
> @Override
> public V put(E key, V value) {
>- if (key == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("key", key);
>
> int ordinal = key.ordinal();
> V previousValue = (V)values[ordinal];
>@@ -76,9 +72,7 @@
> @SuppressWarnings("unchecked")
> @Override
> public V remove(E key) {
>- if (key == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("key", key);
>
> V value = null;
> if (keySet.contains(key)) {
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedList.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedList.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedList.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedList.java Fri Nov 13 13:15:02 2009
>@@ -305,10 +305,7 @@
>
> @Override
> public void insert(T item, int index) {
>- if (index < 0
>- || index > length) {
>- throw new IndexOutOfBoundsException();
>- }
>+ CollectionArgChecks.indexBounds(index, 0, length);
>
> Node<T> next = null;
> Node<T> previous = null;
>@@ -348,10 +345,7 @@
>
> @Override
> public T update(int index, T item) {
>- if (index < 0
>- || index >= length) {
>- throw new IndexOutOfBoundsException();
>- }
>+ CollectionArgChecks.indexBounds(index, 0, length - 1);
>
> // Get the previous item at index
> Node<T> node = getNode(index);
>@@ -409,10 +403,7 @@
>
> @Override
> public Sequence<T> remove(int index, int count) {
>- if (index < 0
>- || index + count > length) {
>- throw new IndexOutOfBoundsException();
>- }
>+ CollectionArgChecks.indexBounds(index, count, 0, length);
>
> LinkedList<T> removed = new LinkedList<T>();
>
>@@ -479,10 +470,7 @@
>
> @Override
> public T get(int index) {
>- if (index < 0
>- || index >= length) {
>- throw new IndexOutOfBoundsException();
>- }
>+ CollectionArgChecks.indexBounds(index, 0, length - 1);
>
> Node<T> node = getNode(index);
> return node.item;
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedQueue.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedQueue.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedQueue.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedQueue.java Fri Nov 13 13:15:02 2009
>@@ -57,7 +57,7 @@
> public T dequeue() {
> int length = linkedList.getLength();
> if (length == 0) {
>- throw new IllegalStateException();
>+ throw new IllegalStateException("queue is empty");
> }
>
> T item = linkedList.remove(length - 1, 1).get(0);
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedStack.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedStack.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedStack.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/LinkedStack.java Fri Nov 13 13:15:02 2009
>@@ -51,7 +51,7 @@
> public T pop() {
> int length = linkedList.getLength();
> if (length == 0) {
>- throw new IllegalStateException();
>+ throw new IllegalStateException("queue is empty");
> }
>
> T item = linkedList.remove(length - 1, 1).get(0);
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedList.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedList.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedList.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedList.java Fri Nov 13 13:15:02 2009
>@@ -19,6 +19,7 @@
> import java.util.Comparator;
> import java.util.Iterator;
>
>+import org.apache.pivot.collections.CollectionArgChecks;
> import org.apache.pivot.collections.List;
> import org.apache.pivot.collections.ListListener;
> import org.apache.pivot.collections.Sequence;
>@@ -72,9 +73,7 @@
> private SynchronizedListListenerList<T> listListeners = new SynchronizedListListenerList<T>();
>
> public SynchronizedList(List<T> list) {
>- if (list == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("list", list);
>
> this.list = list;
> }
>@@ -107,7 +106,7 @@
> public synchronized int remove (T item) {
> int index = indexOf(item);
> if (index == -1) {
>- throw new IllegalArgumentException();
>+ throw new IllegalArgumentException("item not in list");
> }
>
> remove(index, 1);
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedMap.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedMap.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedMap.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedMap.java Fri Nov 13 13:15:02 2009
>@@ -19,6 +19,7 @@
> import java.util.Comparator;
> import java.util.Iterator;
>
>+import org.apache.pivot.collections.CollectionArgChecks;
> import org.apache.pivot.collections.Map;
> import org.apache.pivot.collections.MapListener;
> import org.apache.pivot.util.ImmutableIterator;
>@@ -70,9 +71,7 @@
> private SynchronizedMapListenerList<K, V> mapListeners = new SynchronizedMapListenerList<K, V>();
>
> public SynchronizedMap(Map<K, V> map) {
>- if (map == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("map", map);
>
> this.map = map;
> }
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedQueue.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedQueue.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedQueue.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedQueue.java Fri Nov 13 13:15:02 2009
>@@ -19,6 +19,7 @@
> import java.util.Comparator;
> import java.util.Iterator;
>
>+import org.apache.pivot.collections.CollectionArgChecks;
> import org.apache.pivot.collections.Queue;
> import org.apache.pivot.collections.QueueListener;
> import org.apache.pivot.util.ImmutableIterator;
>@@ -56,9 +57,7 @@
> private SynchronizedQueueListenerList<T> queueListeners = new SynchronizedQueueListenerList<T>();
>
> public SynchronizedQueue(Queue<T> queue) {
>- if (queue == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("queue", queue);
>
> this.queue = queue;
> }
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedSet.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedSet.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedSet.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedSet.java Fri Nov 13 13:15:02 2009
>@@ -19,6 +19,7 @@
> import java.util.Comparator;
> import java.util.Iterator;
>
>+import org.apache.pivot.collections.CollectionArgChecks;
> import org.apache.pivot.collections.Set;
> import org.apache.pivot.collections.SetListener;
> import org.apache.pivot.util.ImmutableIterator;
>@@ -65,9 +66,7 @@
> private SynchronizedSetListenerList<E> setListeners = new SynchronizedSetListenerList<E>();
>
> public SynchronizedSet(Set<E> set) {
>- if (set == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("set", set);
>
> this.set = set;
> }
>
>Modified: incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedStack.java
>URL: http://svn.apache.org/viewvc/incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedStack.java?rev=835842&r1=835841&r2=835842&view=diff
>==============================================================================
>--- incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedStack.java (original)
>+++ incubator/pivot/trunk/core/src/org/apache/pivot/collections/concurrent/SynchronizedStack.java Fri Nov 13 13:15:02 2009
>@@ -19,6 +19,7 @@
> import java.util.Comparator;
> import java.util.Iterator;
>
>+import org.apache.pivot.collections.CollectionArgChecks;
> import org.apache.pivot.collections.Stack;
> import org.apache.pivot.collections.StackListener;
> import org.apache.pivot.util.ImmutableIterator;
>@@ -56,9 +57,7 @@
> private SynchronizedStackListenerList<T> stackListeners = new SynchronizedStackListenerList<T>();
>
> public SynchronizedStack(Stack<T> stack) {
>- if (stack == null) {
>- throw new IllegalArgumentException();
>- }
>+ CollectionArgChecks.notNull("stack", stack);
>
> this.stack = stack;
> }
>
>
>
>