RFR : 7178639 : (XXS) Remove incorrect documentation from Deque.push(E e)

Martin Buchholz martinrb at google.com
Tue Apr 30 01:05:03 UTC 2013


Below is my proposed alternative fix, that also  adds some missing @throws,
reuses some existing wording, and makes small improvements to existing
@throws specs (maintaining these specs is very tedious...)

Index: src/main/java/util/Deque.java
===================================================================
RCS file: /export/home/jsr166/jsr166/jsr166/src/main/java/util/Deque.java,v
retrieving revision 1.24
diff -u -U 15 -r1.24 Deque.java
--- src/main/java/util/Deque.java 11 Feb 2013 17:27:45 -0000 1.24
+++ src/main/java/util/Deque.java 30 Apr 2013 01:01:56 -0000
@@ -155,51 +155,53 @@
  * methods, but instead inherit the identity-based versions from class
  * {@code Object}.
  *
  * <p>This interface is a member of the <a
  * href="{@docRoot}/../technotes/guides/collections/index.html"> Java
Collections
  * Framework</a>.
  *
  * @author Doug Lea
  * @author Josh Bloch
  * @since  1.6
  * @param <E> the type of elements held in this collection
  */
 public interface Deque<E> extends Queue<E> {
     /**
      * Inserts the specified element at the front of this deque if it is
-     * possible to do so immediately without violating capacity
restrictions.
-     * When using a capacity-restricted deque, it is generally preferable
to
-     * use method {@link #offerFirst}.
+     * possible to do so immediately without violating capacity
restrictions,
+     * throwing an {@code IllegalStateException} if no space is currently
+     * available.  When using a capacity-restricted deque, it is generally
+     * preferable to use method {@link #offerFirst}.
      *
      * @param e the element to add
      * @throws IllegalStateException if the element cannot be added at this
      *         time due to capacity restrictions
      * @throws ClassCastException if the class of the specified element
      *         prevents it from being added to this deque
      * @throws NullPointerException if the specified element is null and
this
      *         deque does not permit null elements
      * @throws IllegalArgumentException if some property of the specified
      *         element prevents it from being added to this deque
      */
     void addFirst(E e);

     /**
      * Inserts the specified element at the end of this deque if it is
-     * possible to do so immediately without violating capacity
restrictions.
-     * When using a capacity-restricted deque, it is generally preferable
to
-     * use method {@link #offerLast}.
+     * possible to do so immediately without violating capacity
restrictions,
+     * throwing an {@code IllegalStateException} if no space is currently
+     * available.  When using a capacity-restricted deque, it is generally
+     * preferable to use method {@link #offerLast}.
      *
      * <p>This method is equivalent to {@link #add}.
      *
      * @param e the element to add
      * @throws IllegalStateException if the element cannot be added at this
      *         time due to capacity restrictions
      * @throws ClassCastException if the class of the specified element
      *         prevents it from being added to this deque
      * @throws NullPointerException if the specified element is null and
this
      *         deque does not permit null elements
      * @throws IllegalArgumentException if some property of the specified
      *         element prevents it from being added to this deque
      */
     void addLast(E e);

@@ -441,32 +443,31 @@
      * returns {@code null} if this deque is empty.
      *
      * <p>This method is equivalent to {@link #peekFirst()}.
      *
      * @return the head of the queue represented by this deque, or
      *         {@code null} if this deque is empty
      */
     E peek();


     // *** Stack methods ***

     /**
      * Pushes an element onto the stack represented by this deque (in other
      * words, at the head of this deque) if it is possible to do so
-     * immediately without violating capacity restrictions, returning
-     * {@code true} upon success and throwing an
+     * immediately without violating capacity restrictions, throwing an
      * {@code IllegalStateException} if no space is currently available.
      *
      * <p>This method is equivalent to {@link #addFirst}.
      *
      * @param e the element to push
      * @throws IllegalStateException if the element cannot be added at this
      *         time due to capacity restrictions
      * @throws ClassCastException if the class of the specified element
      *         prevents it from being added to this deque
      * @throws NullPointerException if the specified element is null and
this
      *         deque does not permit null elements
      * @throws IllegalArgumentException if some property of the specified
      *         element prevents it from being added to this deque
      */
     void push(E e);
Index: src/main/java/util/concurrent/BlockingDeque.java
===================================================================
RCS file:
/export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/BlockingDeque.java,v
retrieving revision 1.25
diff -u -U 15 -r1.25 BlockingDeque.java
--- src/main/java/util/concurrent/BlockingDeque.java 11 Feb 2013 17:27:45
-0000 1.25
+++ src/main/java/util/concurrent/BlockingDeque.java 30 Apr 2013 01:01:57
-0000
@@ -596,28 +596,29 @@
      * @return the number of elements in this deque
      */
     public int size();

     /**
      * Returns an iterator over the elements in this deque in proper
sequence.
      * The elements will be returned in order from first (head) to last
(tail).
      *
      * @return an iterator over the elements in this deque in proper
sequence
      */
     Iterator<E> iterator();

     // *** Stack methods ***

     /**
-     * Pushes an element onto the stack represented by this deque.  In
other
-     * words, inserts the element at the front of this deque unless it
would
-     * violate capacity restrictions.
+     * Pushes an element onto the stack represented by this deque (in other
+     * words, at the head of this deque) if it is possible to do so
+     * immediately without violating capacity restrictions, throwing an
+     * {@code IllegalStateException} if no space is currently available.
      *
      * <p>This method is equivalent to {@link #addFirst(Object) addFirst}.
      *
      * @throws IllegalStateException {@inheritDoc}
      * @throws ClassCastException {@inheritDoc}
      * @throws NullPointerException if the specified element is null
      * @throws IllegalArgumentException {@inheritDoc}
      */
     void push(E e);
 }
Index: src/main/java/util/concurrent/ConcurrentLinkedDeque.java
===================================================================
RCS file:
/export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/ConcurrentLinkedDeque.java,v
retrieving revision 1.43
diff -u -U 15 -r1.43 ConcurrentLinkedDeque.java
--- src/main/java/util/concurrent/ConcurrentLinkedDeque.java 27 Mar 2013
19:46:34 -0000 1.43
+++ src/main/java/util/concurrent/ConcurrentLinkedDeque.java 30 Apr 2013
01:01:57 -0000
@@ -989,35 +989,51 @@
     }

     /**
      * Inserts the specified element at the tail of this deque.
      * As the deque is unbounded, this method will never throw
      * {@link IllegalStateException} or return {@code false}.
      *
      * @return {@code true} (as specified by {@link Collection#add})
      * @throws NullPointerException if the specified element is null
      */
     public boolean add(E e) {
         return offerLast(e);
     }

     public E poll()           { return pollFirst(); }
-    public E remove()         { return removeFirst(); }
     public E peek()           { return peekFirst(); }
+
+    /**
+     * @throws NoSuchElementException {@inheritDoc}
+     */
+    public E remove()         { return removeFirst(); }
+
+    /**
+     * @throws NoSuchElementException {@inheritDoc}
+     */
+    public E pop()            { return removeFirst(); }
+
+    /**
+     * @throws NoSuchElementException {@inheritDoc}
+     */
     public E element()        { return getFirst(); }
+
+    /**
+     * @throws NullPointerException {@inheritDoc}
+     */
     public void push(E e)     { addFirst(e); }
-    public E pop()            { return removeFirst(); }

     /**
      * Removes the first element {@code e} such that
      * {@code o.equals(e)}, if such an element exists in this deque.
      * If the deque does not contain the element, it is unchanged.
      *
      * @param o element to be removed from this deque, if present
      * @return {@code true} if the deque contained the specified element
      * @throws NullPointerException if the specified element is null
      */
     public boolean removeFirstOccurrence(Object o) {
         checkNotNull(o);
         for (Node<E> p = first(); p != null; p = succ(p)) {
             E item = p.item;
             if (item != null && o.equals(item) && p.casItem(item, null)) {
Index: src/main/java/util/concurrent/LinkedBlockingDeque.java
===================================================================
RCS file:
/export/home/jsr166/jsr166/jsr166/src/main/java/util/concurrent/LinkedBlockingDeque.java,v
retrieving revision 1.44
diff -u -U 15 -r1.44 LinkedBlockingDeque.java
--- src/main/java/util/concurrent/LinkedBlockingDeque.java 27 Mar 2013
19:46:34 -0000 1.44
+++ src/main/java/util/concurrent/LinkedBlockingDeque.java 30 Apr 2013
01:01:57 -0000
@@ -280,40 +280,40 @@
             unlinkLast();
         } else {
             p.next = n;
             n.prev = p;
             x.item = null;
             // Don't mess with x's links.  They may still be in use by
             // an iterator.
             --count;
             notFull.signal();
         }
     }

     // BlockingDeque methods

     /**
-     * @throws IllegalStateException {@inheritDoc}
-     * @throws NullPointerException  {@inheritDoc}
+     * @throws IllegalStateException if this deque is full
+     * @throws NullPointerException {@inheritDoc}
      */
     public void addFirst(E e) {
         if (!offerFirst(e))
             throw new IllegalStateException("Deque full");
     }

     /**
-     * @throws IllegalStateException {@inheritDoc}
+     * @throws IllegalStateException if this deque is full
      * @throws NullPointerException  {@inheritDoc}
      */
     public void addLast(E e) {
         if (!offerLast(e))
             throw new IllegalStateException("Deque full");
     }

     /**
      * @throws NullPointerException {@inheritDoc}
      */
     public boolean offerFirst(E e) {
         if (e == null) throw new NullPointerException();
         Node<E> node = new Node<E>(e);
         final ReentrantLock lock = this.lock;
         lock.lock();
@@ -588,32 +588,31 @@
             return false;
         } finally {
             lock.unlock();
         }
     }

     // BlockingQueue methods

     /**
      * Inserts the specified element at the end of this deque unless it
would
      * violate capacity restrictions.  When using a capacity-restricted
deque,
      * it is generally preferable to use method {@link #offer(Object)
offer}.
      *
      * <p>This method is equivalent to {@link #addLast}.
      *
-     * @throws IllegalStateException if the element cannot be added at this
-     *         time due to capacity restrictions
+     * @throws IllegalStateException if this deque is full
      * @throws NullPointerException if the specified element is null
      */
     public boolean add(E e) {
         addLast(e);
         return true;
     }

     /**
      * @throws NullPointerException if the specified element is null
      */
     public boolean offer(E e) {
         return offerLast(e);
     }

     /**
@@ -726,32 +725,32 @@
         try {
             int n = Math.min(maxElements, count);
             for (int i = 0; i < n; i++) {
                 c.add(first.item);   // In this order, in case add()
throws.
                 unlinkFirst();
             }
             return n;
         } finally {
             lock.unlock();
         }
     }

     // Stack methods

     /**
-     * @throws IllegalStateException {@inheritDoc}
-     * @throws NullPointerException  {@inheritDoc}
+     * @throws IllegalStateException if this deque is full
+     * @throws NullPointerException {@inheritDoc}
      */
     public void push(E e) {
         addFirst(e);
     }

     /**
      * @throws NoSuchElementException {@inheritDoc}
      */
     public E pop() {
         return removeFirst();
     }

     // Collection methods

     /**
@@ -817,31 +816,31 @@
      * collection, especially when count is close to capacity.
      */

 //     /**
 //      * Adds all of the elements in the specified collection to this
 //      * queue.  Attempts to addAll of a queue to itself result in
 //      * {@code IllegalArgumentException}. Further, the behavior of
 //      * this operation is undefined if the specified collection is
 //      * modified while the operation is in progress.
 //      *
 //      * @param c collection containing elements to be added to this queue
 //      * @return {@code true} if this queue changed as a result of the
call
 //      * @throws ClassCastException            {@inheritDoc}
 //      * @throws NullPointerException          {@inheritDoc}
 //      * @throws IllegalArgumentException      {@inheritDoc}
-//      * @throws IllegalStateException         {@inheritDoc}
+//      * @throws IllegalStateException if this deque is full
 //      * @see #add(Object)
 //      */
 //     public boolean addAll(Collection<? extends E> c) {
 //         if (c == null)
 //             throw new NullPointerException();
 //         if (c == this)
 //             throw new IllegalArgumentException();
 //         final ReentrantLock lock = this.lock;
 //         lock.lock();
 //         try {
 //             boolean modified = false;
 //             for (E e : c)
 //                 if (linkLast(e))
 //                     modified = true;
 //             return modified;



On Mon, Apr 29, 2013 at 4:56 PM, Mike Duigou <mike.duigou at oracle.com> wrote:

> OK, I will wait on that and hopefully Chris can pick it up on the next
> jsr166 sync.
>
> Mike
>
> On Apr 29 2013, at 16:43 , Martin Buchholz wrote:
>
> As always for changes to files maintained in jsr166 CVS, we'd like changes
> to flow through jsr166 CVS into openjdk proper.
>
> In this case, there are some issues with missing exception specs in
> implementing classes.  I'll come up with a diff.
>
> Martin
>
>
> On Fri, Apr 26, 2013 at 5:12 PM, Mike Duigou <mike.duigou at oracle.com>wrote:
>
>> Hello all;
>>
>> A very small change to review.
>>
>> http://cr.openjdk.java.net/~mduigou/JDK-7178639/0/webrev/
>>
>> The change removes some erroneous documentation from the Deque push()
>> method.
>>
>> Mike
>>
>
>
>



More information about the core-libs-dev mailing list