patternjavaModerate

Can someone review my implementation of a KeyValuePair?

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

cansomeonekeyvaluepairimplementationreview

Problem

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;
    }
}

Solution

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, V>
        implements Comparable> {

    private K key;
    private V value;

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

    @Override
    public int compareTo(KeyValuePair 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, V> implements Comparable> {

    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 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());
    }
}

Code Snippets

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;
    }
}

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());
    }
}

Context

StackExchange Code Review Q#32267, answer score: 10

Revisions (0)

No revisions yet.