Chris K
Chris K

Reputation: 12359

Java generics Pair<String, String> stored in HashMap not retrieving key->value properly

Here's Pair.java

import java.lang.*; 
import java.util.*; 

public class Pair<TYPEA, TYPEB> implements Comparable< Pair<TYPEA, TYPEB> > {
  protected final TYPEA Key_;
  protected final TYPEB Value_;

  public Pair(TYPEA key, TYPEB value) {
    Key_   = key;
    Value_ = value;
  }
  public TYPEA getKey() {
    return Key_;
  }
  public TYPEB getValue() {
    return Value_;
  }
  public String toString() {
    System.out.println("in toString()");
    StringBuffer buff = new StringBuffer();
      buff.append("Key: ");
      buff.append(Key_);
      buff.append("\tValue: ");
      buff.append(Value_);
    return(buff.toString() );
  }
  public int compareTo( Pair<TYPEA, TYPEB> p1 ) { 
    System.out.println("in compareTo()");
    if ( null != p1 ) { 
      if ( p1.equals(this) ) { 
        return 0; 
      } else if ( p1.hashCode() > this.hashCode() ) { 
            return 1;
      } else if ( p1.hashCode() < this.hashCode() ) { 
        return -1;  
      }
    }
    return(-1);
  }
  public boolean equals( Pair<TYPEA, TYPEB> p1 ) { 
    System.out.println("in equals()");
    if ( null != p1 ) { 
      if ( p1.Key_.equals( this.Key_ ) && p1.Value_.equals( this.Value_ ) ) { 
        return(true);
      }
    }
    return(false);
  }
  public int hashCode() { 
    int hashCode = Key_.hashCode() + (31 * Value_.hashCode());
    System.out.println("in hashCode() [" + Integer.toString(hashCode) + "]");
    return(hashCode);
  }
}

Here's the testcase:

import java.lang.*; 
import java.util.*;

import junit.framework.*;

public class PairTest extends TestCase { 

  public void testPair() { 
    String key   = new String("key"); 
    String value = new String("asdf"); 

    Pair<String, String> pair = new Pair<String, String>( key, value ); 

    assertTrue( pair.getKey().equals( key ) );
    assertTrue( pair.getValue().equals( value ) );
    assertTrue( pair.equals( new Pair<String, String>(key, value)) );
  }

  public void testPairCollection() { 

    HashMap< Pair<String, String>, String> hm1 = new HashMap<Pair<String,String>, String>(); 

    Pair<String, String> p1 = new Pair<String, String>("Test1", "Value1"); 
       hm1.put(p1, "ONE");  
    Pair<String, String> p2 = new Pair<String, String>("Test1", "Value2"); 
       hm1.put(p2, "TWO");  
    Pair<String, String> p3 = new Pair<String, String>("Test2", "Value1"); 
       hm1.put(p3, "THREE");    
    Pair<String, String> p4 = new Pair<String, String>("Test2", "Value2"); 
       hm1.put(p4, "FOUR"); 
    Pair<String, String> p5 = new Pair<String, String>("Test3", "Value1"); 
       hm1.put(p5, "FIVE"); 
    Pair<String, String> p6 = new Pair<String, String>("Test3", "Value2"); 
       hm1.put(p6, "SIX");  
    Pair<String, String> p7 = new Pair<String, String>("Test3", "Value3"); 
       hm1.put(p7, "SEVEN");    

    assertTrue( hm1.size() == 7 ); 

    Pair<String, String> pSrch = new Pair<String, String>("Test3", "Value3"); 
    assertTrue( p7.equals(pSrch) );
    assertTrue( pSrch.equals(p7) );
    assertTrue( p7.hashCode() == pSrch.hashCode() ); 
    assertTrue( 0 == p7.compareTo( pSrch ) );
    assertTrue( 0 == pSrch.compareTo(p7) );

    System.out.println("starting containsKey search");
    assertTrue( hm1.containsKey( p7 ) );
    System.out.println("starting containsKey search2");
    assertTrue( hm1.containsKey( pSrch ) );
    System.out.println("finishing containsKey search");

    String result = hm1.get( pSrch );
    assertTrue( null != result );
    assertTrue( 0 == result.compareTo("SEVEN"));

  } 
}

Here's my problem, the last hm1.containsKey call should (I naively expect) return the value stored where Pair<"Three", "Three"> is true - I should get a String with a value of "SEVEN". Here is the output:

Running in equals()
in hashCode() [1976956095]
in hashCode() [1976956126]
in hashCode() [1976956096]
in hashCode() [1976956127]
in hashCode() [1976956097]
in hashCode() [1976956128]
in hashCode() [1976956159]
in equals()
in equals()
in hashCode() [1976956159]
in hashCode() [1976956159]
in compareTo()
in equals()
in compareTo()
in equals()
starting containsKey search
in hashCode() [1976956159]
starting containsKey search2
in hashCode() [1976956159]     <--- Bug here?

Never reaches 
          String result = hm1.get( pSrch );

So is both p7.hashCode() and pSrch.hashCode() are equal and p7.equals(pSrch) and pSrch.equals(p7), and hm1.containsValue(p7) == true, I would expect hm1.containsValue(pSrch) would also return true, but it does not. What am I missing?

Upvotes: 3

Views: 46003

Answers (2)

Pavel Feldman
Pavel Feldman

Reputation: 4719

You should have noticed that it does not print "in equals()" after "starting containsKey search2". Also you could debug into HashMap to see that .equals() method is called and returns false. That's because

public boolean equals( Pair<TYPEA, TYPEB> p1 )

does NOT override

public boolean equals(Object obj)

defined in java.lang.Object

Change your code to

  public boolean equals( Object obj ) {
    if (!(obj instanceof Pair)) return false;
    Pair p1 = (Pair) obj;

and it works. You can avoid such bugs in the future by putting @Override annotation before method that you think you are overriding. If you are not actually overriding it, compiler will tell you. This

@Override public boolean equals( Pair<TYPEA, TYPEB> p1 )

causes compilation error. This

@Override public boolean equals( Object obj )

does not. Also good IDE (Intellij IDEA for example) shows which methods are overriden.

Upvotes: 5

erickson
erickson

Reputation: 269797

You need to override the equals method from the java.lang.Object class.

Instead, you've overloaded the method with an additional version that takes a Pair. Totally different method that never gets called. Replace your equals with something like this:

@Override
public boolean equals(Object o) { 
  System.out.println("in equals()");
  if (o instanceof Pair) { 
    Pair<?, ?> p1 = (Pair<?, ?>) o;
    if ( p1.Key_.equals( this.Key_ ) && p1.Value_.equals( this.Value_ ) ) { 
      return(true);
    }
  }
  return(false);
}

To avoid this kind of mistake, use the @Override annotation on methods you intend to act as overrides. You'll get a compile time error when they don't.

Upvotes: 24

Related Questions