On Wed, 20 Mar 2002, Morgan Delagrange wrote: > Sure, we can wait until tomorrow. Just be sure to get it all on the table > at once, so we don't spread out the discussion unnecessarily.
Okay, I've finished my (somewhat hurried and tiring) review for possible non-backwards compatible changes. I'm willing to do the necessary work for all of these. Here's the list of issues and improvements I have found in collections which require non-backwards compatible changes (and thus probably should go in before a 2.0 release if they are to go in at all): 1. AbstractBag seems to be more of a "default map based implementation" and differs in design from the AbstractSet, AbstractMap classes which do not make assumptions about how they might be implemented. To be consistent with JDK AbstractX collections, this should just be providing default implementations that could be used regardless of underlying storage mechanism. For example, the add(Object) method would call the abstract add(Object,int) method passing the object and 1. I think iterator() would need to be the only other abstract method. There's a few ways to fix this. First, reimplement so that only the minimal interfaces need to be implemented, thus matching the style of the JDK AbstractX collections. Second rename to DefaultMapBag or similar so that there isn't a difference in style. Third, a combination of both. The minimum necessary for a 2.0 release is just the rename. If you're confused about what I mean here, I can probably put together a patch this weekend for the third option (a new AbstractBag and a DefaultMapBag). 2. ArrayEnumeration is not fail-fast when a (Object[])null value is passed to the constructor. 3. BeanMap.values() is modifiable, but modifications are not reflected in the BeanMap. Probably should return an unmodifiable collection. 4. BeanMap.keySet() is modifiable. Modifications to it (i.e. removes) will be reflected in the BeanMap, but that's probably not a good idea. Probably should return an unmodifiable collection. 5. DoubleOrderedMap requires its keys and elements to be comparable. I don't think the external API relies on Comparable (just the internal API), so I don't think this must be fixed before a 2.0 release, but I didn't look quite that closely to know for sure. 6. The following classes have "protected" members and/or methods, but the ability to subclass without "corrupting" the internal state of the parent class is suspect: BeanMap, FastArrayList, FastHashMap, FastTreeMap. Fixing these would probably involve making some fields private. My brain power started dying, so I didn't really review the following classes (unfortunate since these are pretty sophisticated classes): CollectionUtils, CursorableLinkedList, ExtendedProperties, SoftRefHashMap There's also a lot of places where I disagree with the implementation of things, but I think it'd probably be too much for me to complain about all of them as well (e.g. MapUtils.getXXX() methods should throw exceptions or return null for things that aren't XXX rather than "massaging" them into XXX or using System.out.println) One other note: package.html probably should be updated before 2.0 release. tired, michael -- To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]> For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
