I have coded a first draft of alternative #1 of the proposal described in the attached email. Feedback is welcome, as well as your thoughts on whether this should be committed.
The source code for the NullObjectComparator class is located at: http://mcwestcorp.com/collections-NullObjectComparator.zip Also in this zip file are an updated ComparatorUtils to add the methods nullObjectLowComparator and nullObjectHighComparator. Also included are unit tests for the new class, and an updated version of the TestAll class (which runs all Comparator unit tests) which also runs the (new) unit test for NullObjectComparator. An example usage of NullObjectComparator is also included: NullObjectComparatorExample In writing the above unit tests, I also came across a possible bug in beanutils BeanComparator. This class does not currently have an equals method. An updated version of the class has been included. -- Brian -- Brian Westrich McWest Corp. email: [EMAIL PROTECTED] P.S. Here is the bugzilla report related to the above issue with BeanComparator: http://nagoya.apache.org/bugzilla/show_bug.cgi?id=27381 -----Original Message----- From: Brian Westrich [mailto:[EMAIL PROTECTED] Sent: Thursday, February 26, 2004 8:44 AM To: [EMAIL PROTECTED] Cc: Brian Westrich [EMAIL PROTECTED] Subject: [COLLECTIONS (Comparator)] Proposal: add parent bean null checking Hello commons collections developers, When using commons comparators to sort collections, I need to handle cases where a null (top level) element is found in the collection (I know this is not a usual error condition, but the sorting code I'm replacing with a Jakarta commons approach does this check so my new code needs to do it as well). The NullComparator class works fine for handling null properties on elements, e.g. ... List comparators = new ArrayList(); comparators.add(new BeanComparator("name", new NullComparator())); Comparator comparator = new ComparatorChain(comparators); Collections.sort(toSort, comparator); // works fine but using NullComparator (with the default constructor) to detect null top level elements currently only works if all elements implement the Comparable interface. Specifically, the following code throws a ClassCastException if one or more list elements do not implement Comparable .... List comparators = new ArrayList(); comparators.add(new NullComparator()); comparators.add(new BeanComparator("name", new NullComparator())); Comparator comparator = new ComparatorChain(comparators); Collections.sort(toSort, comparator); // throws a ClassCastException if all list elements don't implement comparable interface One alternative would be to create a new class (named something like NullBeanComparator or NullObjectComparator) similar to NullComparator except that when both objects were non-null it would return 0, e.g. public int compare(Object o1, Object o2) { if(o1 == o2) { return 0; } if(o1 == null) { return (this.nullsAreHigh ? 1 : -1); } if(o2 == null) { return (this.nullsAreHigh ? -1 : 1); } return 0; } This new class would be appropriate for use as the first comparator in the above comparator chain. One would also add analogous methods to ComparatorUtils such as nullObjectLowComparator and nullObjectHighComparator. Another (less preferable?) alternative would be to change NullComparator.compare(Object, Object) to treat two objects as equal if both are not null and either does not implement the comparable interface. e.g. public int compare(Object o1, Object o2) { if(o1 == o2) { return 0; } if(o1 == null) { return (this.nullsAreHigh ? 1 : -1); } if(o2 == null) { return (this.nullsAreHigh ? -1 : 1); } if ((o1 instanceof Comparable == false) || (o2 instanceof Comparable == false)) return 0; // a line such as this would be added return this.nonNullComparator.compare(o1, o2); } But this latter approach seems problematic when nonNullComparator does not require the compared objects to implement Comparable (for example, when one constructs a NullComparator using a FixedOrderComparator). In such cases it seems preferable to call nonNullComparator.compare(). So I prefer the first alternative over the second. I appreciate hearing people's thoughts on the above alternatives as well as any other possible alternatives. -- Brian _________________________________________________ Brian Westrich McWest Corp. 612-508-1827 ([EMAIL PROTECTED]) www.mcwestcorp.com _________________________________________________ --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
