You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Brian Westrich <bw...@mcwestcorp.com> on 2004/02/26 15:43:46 UTC

[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 (bw@mcwestcorp.com)
www.mcwestcorp.com
_________________________________________________




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


Re: [COLLECTIONS (Comparator)] Proposal: add parent bean null checking

Posted by Stephen Colebourne <sc...@btopenworld.com>.
I loaded your example code and examined the proposed class. However, I don't
think any change is necessary.

Your code:
 List comparators = new ArrayList();
 comparators.add(new NullObjectComparator());
 comparators.add(new BeanComparator("name", new NullComparator()));
 Comparator comparator = new ComparatorChain(comparators);

Alternative:
 Comparator c = new NullComparator(new BeanComparator("name", new
NullComparator()))

Stephen


----- Original Message -----
From: "Brian Westrich" <bw...@mcwestcorp.com>
> 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: bw@mcwestcorp.com
>
>
> 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:bw@mcwestcorp.com]
> Sent: Thursday, February 26, 2004 8:44 AM
> To: commons-dev@jakarta.apache.org
> Cc: Brian Westrich [Bw@Mcwestcorp.Com]
> 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 (bw@mcwestcorp.com)
> www.mcwestcorp.com
> _________________________________________________
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org


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


RE: [COLLECTIONS (Comparator)] Proposal: add parent bean null checking

Posted by Brian Westrich <bw...@mcwestcorp.com>.
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: bw@mcwestcorp.com


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:bw@mcwestcorp.com]
Sent: Thursday, February 26, 2004 8:44 AM
To: commons-dev@jakarta.apache.org
Cc: Brian Westrich [Bw@Mcwestcorp.Com]
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 (bw@mcwestcorp.com)
www.mcwestcorp.com
_________________________________________________




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