commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From m..@apache.org
Subject cvs commit: jakarta-commons/collections/src/java/org/apache/commons/collections AbstractBag.java Bag.java
Date Fri, 22 Feb 2002 04:39:53 GMT
mas         02/02/21 20:39:53

  Modified:    collections/src/java/org/apache/commons/collections
                        AbstractBag.java Bag.java
  Log:
  Fixed the following Bag related problems:
  
   - In Bag.java, the statement "If the bag contains less than i occurences,
  the item item will be removed from the unique set" implies that if the bag
  contains 5 occurences and i is 5, (5 is not less than 5) then the item will
  not be removed from the unique set, even though there should be no more
  occurances in the bag.
  
   - In AbstractBag.java, the documentation does not specify exactly what a
  subcless needs to do to extend AbstractBag to make a concrete subclass.
  
   - AbstractBag.add(Object,int) has two calls to getCount(o), when only one
  is necessary.  This wastes a few cycles to perform method invocations, a
  map lookup, a cast, and a few comparisons.
  
   - The AbstractBag.equals(Object) method will incorrectly throw a
  NullPointerException if a null value is passed.  The Object.equals(Object)
  API specifies "For any non-null reference value x, x.equals(null) should
  return false".
  
   - The AbstractBag.equals(Object) method will only work if the object
  passed in extends AbstractMap or implements Map.  Neither of these facts
  is documented, and neither is correct.  The contract for
  Object.equals(Object) states: "for any reference values x and y,
  x.equals(y) should return true if and only if y.equals(x) returns true. ".
  Returning true when the argument is a Map is incorrect since he reverse
  (the map checking to see if its equal to the bag) will most certainly be
  false.  The same can be said for AbstractMap.  A subclass of AbstractMap
  may add extra data to be stored within the Bag that must also be compared
  for them to be equal.  The reverse comparison (specialized subclass equals
  basic abstract bap) will fail.  You can read more about this in a three-
  year old, but still valid java world article:
  http://www.javaworld.com/javaworld/jw-01-1999/jw-01-object.html
  
   - if AbstractBag.remove(Object,int) is called passing in 0 as the number
  to remove and specifying an object that exists in the bag, true will be
  returned from the method. Per the Bag API specification, true should only
  return when an object is actually removed.  Since no objects are removed,
  false should be returned instead. Additionally, if a negative number is
  specified, not only is the object not removed, but object(s) are *added*
  (well, in the sense that it is equivalent of calling add(o, -i))
  
   - the uniqueSet() method returns the set of unique objects, however the
  set is modifiable.  If the underlying map implementation has the set
  backed by the map (as per the map contract -- so it should), then elements
  can be removed from the unique set and have them removed from the
  underlying map as well.  This causes consistency problems with the Bag
  since _total will then be incorrect.
  
   - in extractList(), getCount(current) is called each time through the
  inner loop, adding lots of extra overhead.  Reversing the loop (starting
  at the count and going down to 0) eliminates the excess overhead.
  
  Revision  Changes    Path
  1.3       +20 -16    jakarta-commons/collections/src/java/org/apache/commons/collections/AbstractBag.java
  
  Index: AbstractBag.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/AbstractBag.java,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- AbstractBag.java	10 Feb 2002 08:07:42 -0000	1.2
  +++ AbstractBag.java	22 Feb 2002 04:39:53 -0000	1.3
  @@ -1,7 +1,7 @@
   /*
  - * $Header: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/AbstractBag.java,v
1.2 2002/02/10 08:07:42 jstrachan Exp $
  - * $Revision: 1.2 $
  - * $Date: 2002/02/10 08:07:42 $
  + * $Header: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/AbstractBag.java,v
1.3 2002/02/22 04:39:53 mas Exp $
  + * $Revision: 1.3 $
  + * $Date: 2002/02/22 04:39:53 $
    *
    * ====================================================================
    *
  @@ -63,6 +63,7 @@
   
   import java.util.ArrayList;
   import java.util.Collection;
  +import java.util.Collections;
   import java.util.ConcurrentModificationException;
   import java.util.Iterator;
   import java.util.List;
  @@ -73,8 +74,12 @@
   /**
    * This class provides a skeletal implementation of the {@link Bag}
    * interface to minimize the effort required for target implementations.
  + * Subclasses need only to call {@link #setMap(Map)} in their constructor 
  + * specifying a map instance that will be used to store the contents of 
  + * the bag. 
    *
    * @author Chuck Burdick
  + * @author <a href="michael@iammichael.org">Michael Smith</a>
    **/
   public abstract class AbstractBag implements Bag {
      private Map _map = null;
  @@ -91,7 +96,7 @@
            int count = (i + getCount(o));
            _map.put(o, new Integer(count));
            _total += i;
  -         return (getCount(o) == i);
  +         return (count == i);
         } else {
            return false;
         }
  @@ -139,13 +144,9 @@
      }
   
      public boolean equals(Object o) {
  -      boolean result = false;
  -      if (o instanceof AbstractBag) {
  -         result = _map.equals(((AbstractBag)o).getMap());
  -      } else if (o instanceof Map) {
  -         result = _map.equals((Map)o);
  -      }
  -      return result;
  +      return (o == this || 
  +              (o != null && o.getClass().equals(this.getClass()) &&
  +               ((AbstractBag)o)._map.equals(this._map)));
      }
   
      public int hashCode() {
  @@ -203,12 +204,15 @@
         _mods++;
         boolean result = false;
         int count = getCount(o);
  -      if (count > i) {
  +      if (i <= 0) {
  +         result = false;
  +      } else if (count > i) {
            _map.put(o, new Integer(count - i));
            result = true;
            _total -= i;
  -      } else {
  -         result = uniqueSet().remove(o);
  +      } else { // count > 0 && count <= i  
  +         // need to remove all
  +         result = (_map.remove(o) != null);
            _total -= count;
         }
         return result;
  @@ -274,7 +278,7 @@
      }
   
      public Set uniqueSet() {
  -      return _map.keySet();
  +      return Collections.unmodifiableSet(_map.keySet());
      }
   
      public int size() {
  @@ -316,7 +320,7 @@
         Iterator i = uniqueSet().iterator();
         while (i.hasNext()) {
            Object current = i.next();
  -         for (int index = 0; index < getCount(current); index++) {
  +         for (int index = getCount(current); index > 0; index--) {
               result.add(current);
            }
         }
  
  
  
  1.3       +4 -4      jakarta-commons/collections/src/java/org/apache/commons/collections/Bag.java
  
  Index: Bag.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/Bag.java,v
  retrieving revision 1.2
  retrieving revision 1.3
  diff -u -r1.2 -r1.3
  --- Bag.java	10 Feb 2002 08:07:42 -0000	1.2
  +++ Bag.java	22 Feb 2002 04:39:53 -0000	1.3
  @@ -1,7 +1,7 @@
   /*
  - * $Header: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/Bag.java,v
1.2 2002/02/10 08:07:42 jstrachan Exp $
  - * $Revision: 1.2 $
  - * $Date: 2002/02/10 08:07:42 $
  + * $Header: /home/cvs/jakarta-commons/collections/src/java/org/apache/commons/collections/Bag.java,v
1.3 2002/02/22 04:39:53 mas Exp $
  + * $Revision: 1.3 $
  + * $Date: 2002/02/22 04:39:53 $
    *
    * ====================================================================
    *
  @@ -113,7 +113,7 @@
   
      /**
       * Remove the given number of occurrences from the bag. If the bag
  -    * contains less than <code>i</code> occurrences, the item will be
  +    * contains <code>i</code> occurrences or less, the item will be
       * removed from the {@link #uniqueSet}.
       * @see #getCount
       * @see #remove(Object)
  
  
  

--
To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>


Mime
View raw message