[Comparable] problem with ording classes

[Klaus Jesper]

Hi, I have a problem to with sorting some elements.

I have a class Test1 which implements Comparable and so has as compareTo-method:

public class Test1 implements Comparable(){
 
public orderNo = 1;
 
[...]
  public int compareTo(Object o) {
    int returnValue = -1;
    if (!(o instanceof Test1)){
      returnValue = 1;
    }else{
      Test1 compareMe = (Test1)o;
      if (this.orderNo >compareMe.orderNo)
        returnValue = 1;
      else
        returnValue = -1;
    }
    return returnValue;
  }
}

Next I have class Test2, that has a relation to Test1. Every Test2 has one associated Test1. To order a set of Test2-objects I want to use the associated Test1-objects (by comparing them throw compareTo()).

public class Test2 implements Comparable(){
 
[...]
  public int compareTo(Object o) {
    int returnValue = 2;
      if (!(o instanceof Test2 )){
        returnValue = 1;
      }else{
        Test2 compareMe = (Test2 )o;
returnValue = this.Test1.compareTo(compareMe.Test1);
      }
    return returnValue;
  }
}

Now I want to order a Set of Test2 with the help of their associated Test1-class. But it doesn't work:

Set setOfTest2 = someOtherObject.getTest2Objects();
SortedSet setOfTest2Sorted= new TreeSet(setOfTest2);

Any ideas what I'm doing wrong?

Klaus
[ January 30, 2005: Message edited by: Klaus Jesper ]

[David Harkness]

Originally posted by Klaus Jesper:
Now I want to order a Set of Test2 with the help of their associated Test1-class. But it doesn't work

By this do you mean

You get a compile error

You get a runtime error (exception)

The program runs but the ordering is incorrect

??? I like a good puzzle, but you have to give us all the pieces!

This expression

this.Test1.compareTo(compareMe.Test1)

looks suspicious. Does Test2 contain the following member field?

Test1 test1;

If not, Test2 doesn't really contain a Test1. If so, you need to reference Test2's field name -- not the name of the class the field references.

[Klaus Jesper]

Sorry for my inaccuracy. "C: The program runs but the ordering is incorrect" is the problem the occurs.

The correct code is:

public class Test2 implements Comparable(){
 
public Test1 test1;
 
[...]
  public int compareTo(Object o) {
    int returnValue = 2;
      if (!(o instanceof Test2 )){
        returnValue = 1;
      }else{
        Test2 compareMe = (Test2 )o;
returnValue = this.test1.compareTo(compareMe.test1);
      }
    return returnValue;
  }
}

What I want to do is to sort a set of Test2-objects by using the compareTo-method of the associated Test1-object.
[ January 31, 2005: Message edited by: Klaus Jesper ]

[Jeroen Wenting]

That's not what Comparable is designed for.
It's designed to determine the natural ordering for objects of the same type, not to provide a means in one class to order objects from another.

[Stan James]

But the natural order of Test2 objects is based on the values of their Test1 fields. The code snippet really looks just fine.

Klaus, have you sorted a bunch of Test1 objects to verify their compareTo() is doing what you want? What type is orderNo? Is it an object that should be using compareTo() again instead of ">"?

[Jeroen Wenting]

In that case the Test2 class should implement Comparable and have that test on the Test1 field, not like this with Test1 implementing Comparable.

[David Harkness]

Originally posted by Jeroen Wenting:
In that case the Test2 class should implement Comparable and have that test on the Test1 field, not like this with Test1 implementing Comparable.

I disagree. What if Test2s were being ordered by a String field in Test2? Would you reimplement String-comparison inside Test2 or use String's compareTo() method?

Klaus, I suspect that in retyping those classes here you have managed to fix the problem. In other words, you probably have a typo in the originals. Why do I say this? Because I just ran your code (adding only constructors) and it works beautifully.

public class Test1 implements Comparable
{
  public int orderNo = 1;
 
  public Test1 ( int id ) {
    this.orderNo = id;
  }
  
  public int compareTo(Object o) {
    int returnValue = -1;
    if (!(o instanceof Test1)){
      returnValue = 1;
    }else{
      Test1 compareMe = (Test1)o;
      if (this.orderNo >compareMe.orderNo)
        returnValue = 1;
      else
        returnValue = -1;
    }
    return returnValue;
  }
}
 
import java.util.*;
 
public class Test2 implements Comparable
{
  public Test1 test1;
 
  public Test2 ( int id ) {
    this.test1 = new Test1(id);
  }
  
  public int compareTo(Object o) {
    int returnValue = 2;
      if (!(o instanceof Test2 )){
        returnValue = 1;
      }else{
        Test2 compareMe = (Test2 )o;
returnValue = this.test1.compareTo(compareMe.test1);
      }
    return returnValue;
  }
 
    public static void main ( String [] args )
    {
        ArrayList list = new ArrayList();
        list.add(new Test2(5));
        list.add(new Test2(2));
        list.add(new Test2(7));
        list.add(new Test2(8));
        list.add(new Test2(3));
        list.add(new Test2(1));
        list.add(new Test2(4));
        list.add(new Test2(9));
        list.add(new Test2(6));
        
        SortedSet set = new TreeSet(list);
        for ( Iterator i = set.iterator() ; i.hasNext() ;  ) {
            System.out.println(((Test2) i.next()).test1.orderNo);
        }
    }
}

The output is 1 ... 9, each on its own line, as expected.

One thought. If you allow duplicate order numbers, you'll get incorrect behavior. You need to initialize returnValue to 0 in Test1 otherwise you return -1 for equal orderNo values. I assumed you didn't have duplicates.

[Jeroen Wenting]

you call compareTo() on String from inside the compareTo() in your Test2 class.
You can do the same thing here. But you're NOT supposed to expect to be able to compare 2 Test2 objects from inside a Test1 instance which is what the OP seems to be trying to achieve.

So you'd get something like

class Test2
{
 Test1 t;
 ...
 public int compareTo(Object o)
 {
   ...
   int result = t.compareTo(((Test2)o).t);
   ...
   return result;
 }
}
 
class Test1
{
  int i;
  ...
  public int compareTo(Object o)
  {
    ...
    return i-((Test1)o).i;
  }
}

[Klaus Jesper]

Thanks for all answers!

@Stan: My orderNo is of type int.

@all: I'm sorry, but indeed it has been a stupid copy&paste-mistake by me.

In the line that should be

if (!(o instanceof Test2 )){
  returnValue = 1;
}

I actually didn't check for class Test2 but for another class. So I always got 1 as returnValue because the if-statement has been always true.

Thanks again for your great suport!

Klaus

[David Harkness]

Originally posted by Jeroen Wenting:
you call compareTo() on String from inside the compareTo() in your Test2 class.

That's exactly what Klaus is doing; just replace "String" with "Test1". In fact, the code you posted is just a shorter version of Klaus's.

Klaus, this is one reason I recommend pasting your broken code directly into your post when you can. I've seen this twice in the past week now, and it will make you go nuts.