Re: cvs commit: jakarta-commons-sandbox/util/src/java/org/apache/commons/util/compare ComparableComparator.java

[ba...@generationjava.com]

Sounds valid. As far as the contract goes, how about if a
class-cast-exception is thrown in both cases, or if another 'illegal
input' decision is taken, ie) returning 0.

As for its use, consider the following code:

        Comparator cc = new ComparableComparator();
        registry.register(java.lang.Comparable.class, cc);

        // Do all these comparables to increase lookup speed
        registry.register(java.lang.String.class, cc);
        registry.register(java.lang.Number.class, cc);
        registry.register(java.io.File.class, cc);
        registry.register(java.lang.Character.class, cc);
        registry.register(java.lang.Byte.class, cc);

        registry.register(java.net.URL.class, new UrlComparator());
        registry.register(org.apache.commons.mail.Email.class, new EmailComparator());
        registry.register(com.oreilly.book.Isbn.class, new IsbnComparator());

This is an excerpt from a class I hope to add soon, ObjectComparator. This
comparator handles comparison on any object (or attempts to). Making the
Comparable interface special makes it impossible to override the
comparator for java.lang.String later with NumericStringComparator.

The next step is BeanComparator. This one is adorable [opinion]. It allows
me to have a Person object with address object which has a state field. I
can then sort by "address.state" on my collection of Person's, and not
even worry about the type if it's one that ObjectComparator
(BeanComparator's default internal comparator) can easily handle.

"address.line[3]", "address[home].state" and just "[1]" are also legal.
The latter sorts on the 1th index of a collection of arrays. My reason for
holding this back at the moment is that it relies on my own
bean-reflection library and I believe there's already one in jakarta.

Apologies if that all doesn't seem cool. It's one of those components that
got written as an itch, and then suddenly started to pay back many times.

Please let me know if making it throw an illegalargumentexception or some
such in both cases would be a good enough fix.

Bay

> This comparator does not implement the contract in Comparable, which
> requires the inverse comparison to have the inverse result.  That is,
>
> sign(compare(o1, o2)) = -(sign(compare(o2, o1)))
>
> which has a corollary that compare(x,y) should only throw an exception if
> and only it compare(y,x) throws an exception.
>
> Don't see what I mean?  consider o1 implements Comparable, and o2 does
> not.  compare(o1, o2) will probably throw a ClassCastException because the
> implementation of compareTo in o1 will not be able to convert o2 to an
> object suitable for comparing.  The reverse, compare(o2, o1) will return
> 0.
>
> I really don't see the need for this comparator in the first place, but at
> the very minimum, the fact that this comparator violates the Comparator
> contract should be documented and made obvious to any user of the class.
> Without such documentation, I will -1 its inclusion.
>
> regards,
> michael
>
>
>
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: cvs commit: jakarta-commons-sandbox/util/src/java/org/apache/commons/util/compare ComparableComparator.java

[ba...@generationjava.com]

> IllegalArgumentException is a bit weird here in this context.  While I
> agree it it probably the most appropriate exception when the arguments
> aren't comparable, the Comparator javadoc declares it throws a
> ClassCastException "if the arguments' types prevent it from being
> compared".  I'm ok with the IllegalArgument, but you should still add
> javadoc to declare it since it isn't declared in the Comparator's API
> docs.

Ack. You're right. Change made to ClassCastException.


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: cvs commit: jakarta-commons-sandbox/util/src/java/org/apache/commons/util/compare ComparableComparator.java

["Michael A. Smith" <mi...@iammichael.org>]

On Thu, 21 Feb 2002 bayard@generationjava.com wrote:
> And so I had no reason not to add it :) See latest cvs commit message. The
> only change I made in your code is that I return result1 rather than -1 or
> 1. Maintaining the size might be important I guess.

IllegalArgumentException is a bit weird here in this context.  While I
agree it it probably the most appropriate exception when the arguments
aren't comparable, the Comparator javadoc declares it throws a
ClassCastException "if the arguments' types prevent it from being
compared".  I'm ok with the IllegalArgument, but you should still add
javadoc to declare it since it isn't declared in the Comparator's API 
docs.

As for returning result1 rather than 1 or -1, it doesn't matter to me.  

regards,
michael


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: cvs commit: jakarta-commons-sandbox/util/src/java/org/apache/commons/util/compare ComparableComparator.java

[ba...@generationjava.com]

On Thu, 21 Feb 2002 bayard@generationjava.com wrote:

> I've also not gone for your ultra paranoid option. It is a good idea
> though I believe.

And so I had no reason not to add it :) See latest cvs commit message. The
only change I made in your code is that I return result1 rather than -1 or
1. Maintaining the size might be important I guess.

Thanks for the pointers,

Bay


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: cvs commit: jakarta-commons-sandbox/util/src/java/org/apache/commons/util/compare ComparableComparator.java

[ba...@generationjava.com]

Let me know how that latest change looks Michael. I took a slightly longer
route, mainly because the medium of email gave me time to sit and twiddle
my thumbs.

I've also not gone for your ultra paranoid option. It is a good idea
though I believe.

Bay

On Wed, 20 Feb 2002, Michael A. Smith wrote:

> On Thu, 21 Feb 2002 bayard@generationjava.com wrote:
> > Sounds valid. As far as the contract goes, how about if a
> > class-cast-exception is thrown in both cases, or if another 'illegal
> > input' decision is taken, ie) returning 0.
>
> Yup.  I'd be fine with:
>
>   int compare(Object o1, Object o2) {
>     return ((Comparable)o1).compareTo((Comparable)o2);
>   }
>
> With that, you're placing the burden on the comparable object to ensure it
> implements the compareTo method with regards to the Comparable contract,
> rather than keeping the burden of the Comparator contract.  Then, it's not
> your problem.  :)
>
> If you really want to be paranoid, you could do both and make sure they
> return inverses:
>
>   int compare(Object o1, Object o2) {
>     int result1 = ((Comparable)o1).compareTo(o2);
>     int result2 = ((Comparable)o2).compareTo(o1);
>
>     if(result1 == 0 && result2 == 0) return 0;
>     if(result1 < 0 && result2 > 0) return -1;
>     if(result1 > 0 && result2 < 0) return 1;
>
>     // results inconsistent
>     throw new ClassCastException("o1 not comparable to o2");
>   }
>
> that may be a bit too paranoid though.
>
> > As for its use, consider the following code:
>
> yeah, that's how I figured it would be used.  :)
>
> regards,
> Michael
>
>
>
> --
> To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
> For additional commands, e-mail: <ma...@jakarta.apache.org>
>
>


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>

Re: cvs commit: jakarta-commons-sandbox/util/src/java/org/apache/commons/util/compare ComparableComparator.java

["Michael A. Smith" <mi...@iammichael.org>]

On Thu, 21 Feb 2002 bayard@generationjava.com wrote:
> Sounds valid. As far as the contract goes, how about if a
> class-cast-exception is thrown in both cases, or if another 'illegal
> input' decision is taken, ie) returning 0.

Yup.  I'd be fine with:

  int compare(Object o1, Object o2) {
    return ((Comparable)o1).compareTo((Comparable)o2);
  }

With that, you're placing the burden on the comparable object to ensure it 
implements the compareTo method with regards to the Comparable contract, 
rather than keeping the burden of the Comparator contract.  Then, it's not 
your problem.  :)

If you really want to be paranoid, you could do both and make sure they 
return inverses:

  int compare(Object o1, Object o2) {
    int result1 = ((Comparable)o1).compareTo(o2);
    int result2 = ((Comparable)o2).compareTo(o1);

    if(result1 == 0 && result2 == 0) return 0;
    if(result1 < 0 && result2 > 0) return -1;
    if(result1 > 0 && result2 < 0) return 1;
 
    // results inconsistent
    throw new ClassCastException("o1 not comparable to o2");
  }

that may be a bit too paranoid though.

> As for its use, consider the following code:

yeah, that's how I figured it would be used.  :)

regards,
Michael



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>