Implementing the comparable interface

Question

I just found this exam question and cannot figure it out :

The following depicts a contrived partial class which implements the Comparable interface. The only purpose of this contrived class is comparing its instances with a given string.

There are two things we need to fill in in the class to finish it. Here is the class :

public class PrivateComparableClass // FILL IN PART 1 { 
   private String thing;

    public PrivateComparableClass(String thing) {
     this.thing=thing;
    }
   //FILL IN PART 2
}

I am assuming part 1 simply corresponds to :

public class PrivateComparableClass implements Comparable {

And part 2, I assume he is expecting an implementation of the compareTo method, but I don't really know how to properly go about implementing this:

public static int compareTo() {
  if this.thing.equals(thing){
  return 1;
  } else {
    return -1;
  }
}

How would I go about fixing this?

Solution 1

First of all, part 1 should really be:

public class PrivateComparableClass implements Comparable<PrivateComparableClass> {

As to part 2, if thing is the only data member in the class, you can simply piggyback on String.compareTo:

public int compareTo(PrivateComparableClass rhs) {
  return this.thing.compareTo(rhs.thing);
}

I recommend that you read up on how compareTo is meant to work (there are three possible outcomes: less than, equal to and greater than).

Solution 2

To expand a bit:

Comparator functions typically take two arguments (let's call them A and B) and follow the convention of returning

• -1 if A < B
• 0 if A == B
• 1 if A > B

Also, compareTo should not be declared 'static' if you are using an instance variable.

Solution 3

First, the Comparable interface is generic; your declarations should specify a type parameter:

public class PrivateComparableClass 
  implements Comparable<PrivateComparableClass> {

Then, you should compare the thing members of the class in a compareTo() method (which is an instance method, not a class member).

@Override
public final int compareTo(PrivateComparableClass that) {
  return this.thing.compareTo(that.thing);
}

A well-behaved Comparable should implement an equals() method that is consistent with its compareTo() method:

@Override
public final boolean equals(Object obj) {
  if (obj == this)
    return true;
  if (!(obj instanceof PrivateComparableClass))
    return false;
  return compareTo((PrivateComparableClass) obj) == 0;
}

And, when you override equals(), you need to override hashCode() too:

@Override
public final int hashCode() {
  return thing.hashCode();
}

If thing is truly allowed to be null, suitable null-checking behavior should be added to each method.

Comments

[Messa]

I think it shouldn't be static :)

[NPE]

@Messa: Good catch, thanks! Copy-and-paste error on my part. Fixed now.

[Marcelo]

In your equals override your last line should be: return compareTo((PrivateComparableClass) obj) == 0;