okaywit / guava-libraries

Automatically exported from code.google.com/p/guava-libraries
Apache License 2.0
0 stars 0 forks source link

MinMaxPriorityQueue.add violates java.util.Queue.add contract #1482

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The contract for java.util.Queue.add(E) states: Inserts the specified element 
into this queue if it is possible to do so immediately without violating 
capacity restrictions, returning true upon success and throwing an 
*IllegalStateException* if no space is currently available.

MinMaxPriorityQueue.add(E) states: Adds the given element to this queue. If 
this queue has a maximum size, after adding element the queue will 
automatically evict its greatest element (according to its comparator), which 
may be element itself.

The description of the add method in MinMaxPriorityQueue gives implementation 
details, but at the end of the day evicting the element which was just added is 
the same as rejecting it.  So I think it violates the Queue.add contract.

Consider the following client code (and please ignore it's stupidity).

public static void add5(final Queue<Integer> queue){
   try{
      queue.add(5);
      System.out.println("Added 5 to the queue");
   }catch(IllegalStateException e){
      System.err.println("Cannot add 5, queue full");
   }
}

The method add5(Queue) is agnostic to what kind of queue it gets.

If for example it is given a MinMaxPriorityQueue with maximumSize of 3 
containing elements [1,2,3] the integer 5 will be immediately evicted.  But the 
method will succeed and print "Added 5 to the queue".

Original issue reported on code.google.com by leonov@gmail.com on 19 Jul 2013 at 7:46

GoogleCodeExporter commented 9 years ago
None of this was unknown to us when we created it; we're aware that the Queue 
contract wasn't written with eviction in mind. 

If we allowed add() to throw an exception when there is an immediate eviction, 
most use cases for the class would simply have to catch that and ignore it. And 
it would *still* be incorrect, as we'd be saying the element was rejected 
because the queue was "full", which is not exactly the correct reason.

Original comment by kevinb@google.com on 19 Jul 2013 at 9:01

GoogleCodeExporter commented 9 years ago
This issue has been migrated to GitHub.

It can be found at https://github.com/google/guava/issues/<issue id>

Original comment by cgdecker@google.com on 1 Nov 2014 at 4:12

GoogleCodeExporter commented 9 years ago

Original comment by cgdecker@google.com on 3 Nov 2014 at 9:08