Can someone review my implementation of a KeyValuePair?

Question

public class KeyValuePair implements Comparable {

    private Comparable key;
    private Object value;

    public KeyValuePair(Comparable key, Object value) {
    this.key = key;
    this.value = value;
    }

    @Override
    public int compareTo(Object item) throws ClassCastException {
    if (!(item instanceof KeyValuePair)) {
        throw new ClassCastException("A KeyValuePair object was expected");
    }
    return this.key.compareTo(((KeyValuePair) item).key);
    }

    public Comparable getKey() {
    return this.key;
    }

    public Object getValue() {
    return this.value;
    }
}

Answer 1

You shouldn't need to throw ClassCastException explicitly; it would be thrown automatically by the JVM if the cast actually fails.

However, you shouldn't have to deal with casting at all if you use Java Generics properly.

public class KeyValuePair<K extends Comparable<K>, V>
        implements Comparable<KeyValuePair<K, V>> {

    private K key;
    private V value;

    public KeyValuePair(K key, V value) {
        this.key = key;
        this.value = value;
    }

    @Override
    public int compareTo(KeyValuePair<K, V> item) {
        return this.key.compareTo(item.key);
    }

    public K getKey() {
        return this.key;
    }

    public V getValue() {
        return this.value;
    }
}

Edit: As @ortang points out, there's more work to be done! Since the key and value are immutable, marking them final is helpful. Also, the class needs to override .equals(), and when that happens, .hashCode() needs to be overridden too. Finally, it may be worthwhile to handle some odd cases in .compareTo().

public class KeyValuePair<K extends Comparable<K>, V> implements Comparable<KeyValuePair<K, V>> {

    private final K key;
    private final V value;

    public KeyValuePair(K key, V value) {
        this.key = key;
        this.value = value;
    }

    // Arbitrarily decide to sort null keys first.  Values are disregarded for
    // the purposes of comparison.
    @Override
    public int compareTo(KeyValuePair<K, V> item) {
        if (this.key == null) {
            return (item.key == null) ? 0 : -1;
        } else {
            return this.key.compareTo(item.key);
        }
    }

    public K getKey() {
        return this.key;
    }

    public V getValue() {
        return this.value;
    }

    @Override
    public boolean equals(Object other) {
        if (other == null || !this.getClass().equals(other.getClass())) {
            return false;
        }
        KeyValuePair item = (KeyValuePair)other;
        return (this.key == null   ? item.key == null   : this.key.equals(item.key)) &&
               (this.value == null ? item.value == null : this.value.equals(item.value));
    }

    @Override
    public int hashCode() {
        return ((this.key == null)   ? 0 : this.key.hashCode()) ^
               ((this.value == null) ? 0 : this.value.hashCode());
    }
}