hash code HashMap

Question

I am trying to learn Java. The Eric Roberts text, "The Art and Science of Java" has a programming assignment where we simulate a flight booking console. I wanted to 'class it up' by using a City class where just a City String would do. It only has one field, name, which is a String, but I am trying to learn how to use classes.

Anyway, so then I had to override the equals method in the City class to avoid getting duplicates. So then I had to override the hashCode method.

Now my HashMap<City,ArrayList<Flight>> isn't working. It can't find certain values and it still permits duplicate keys.

My City equals and hashCode overrides are as follows. Can anybody see why my HashMap is going wrong?

/* (non-Javadoc)
 * @see java.lang.Object#equals(java.lang.Object)
 */
@Override
public boolean equals(Object that) {
    // TODO Auto-generated method stub
    if ( this == that ) return true;
    if ( !( that instanceof City) ) return false;
    City aThat = (City) that;
    return (name == aThat.name );
}

@Override
public int hashCode() {
    // TODO Auto-generated method stub
    return name.hashCode();
}

The problem is how you compare String variables: "name == aThat.name". By using "==", you are just comparing the object reference not the content, the same as "this == that". To compare String objects' content, you need the euquals* method of String object. Unlike C++, every object that you can use in code is just the reference of that object and there is no operator overloading in Java either. — JackyZhu, Aug 26 '13 at 01:30

nachokk · Answer 1 · 2013-08-26T02:11:03.167

For object comparation use equals() instead of == , cause == compares reference values to determine if they are pointing to the same object.

@Override
public boolean equals(Object that) {
    //more code
    return (name.equals(aThat.name) );
}

By the way your hashCode() it's also bad cause your name could be null and you will get a NullPointerException.

@Override
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + ((name == null) ? 0 : name.hashCode());
        return result;
    }

And at last advice , i don't recommend you to use as a key in a hash structure like your hashMap mutable objects cause its hashCode() will change and unexpected behaviour can happen. It's better to use inmutable objects as key. If City class is inmutable then it's ok, but if it's not then change it.

`==` compares reference values *to determine* if they are pointing to the same *object.* -1 for sloppiness. — user207421, Aug 26 '13 at 02:08

ug_ · Answer 2 · 2013-08-26T01:39:14.217

-1

When checking if values of objects are the same dont use ==. Instead use .equals, In your equals method change == to .equals,

For instance

String str1 = "Foo bar";
String str2 = "Foo bar";
str1 == str2 // not always true!
str1.equals(str2) // very true

@Override
public boolean equals(Object that) {
    // TODO Auto-generated method stub
    if ( this == that ) return true;
    if ( !( that instanceof City) ) return false;
    City aThat = (City) that;
    return (name.equals(aThat.name) );   // <--- see edit 
}

edited Aug 26 '13 at 01:39

answered Aug 26 '13 at 01:00

ug_

11,267
2
35
52

-1 for incorrect description of usage of ==, and removal of actually correct line from OP's source code. There are valid use for == when comparing object reference, and the line in OP's code is precisely a valid case. Normally when we implements `equals()`, the very first thing to check is whether the incoming obj ref is referring same object as `this`, if so, they are equals and we can skip all those subsequent heavy checking. – Adrian Shum Aug 26 '13 at 01:14
Well I didnt want to go into GREAT detail of how == works as he is obviously a newcomer to java, im sure he/she is capable of looking into it further if they would like. And I dont belive .equals on a String object should be considered heavy checking. – ug_ Aug 26 '13 at 01:20
1

As I said : 1. The statement 'should never use ==' is simply incorrect, nothing to argue. 2. Check of input obj ref with this is simply the very very normal practice of implementing `equals()`, asking OP to remove that line is simply misleading. I think there are better way to simplify the answer, rather that giving incorrect statements. – Adrian Shum Aug 26 '13 at 01:24
You are correct I overlooked the first sentence of my response. – ug_ Aug 26 '13 at 01:25
2

Removing the `this == that` optimization is a bad idea (empirically) – Stephen C Aug 26 '13 at 01:36
1

Your line `str1 == str2 // not always true!` is, well, not true. – Dawood ibn Kareem Aug 26 '13 at 01:46
removed -1 for the main criticial problems in answer resolved. To elaborate on Davaid's comment: both str1 and str2 is referring to a string literal, which is going to be the same instance in string pool, which is always true in this case. Suggest @ns47731 to change the answer to be something like `String str1= new String("Foo bar");` which can better demonstrate the idea – Adrian Shum Aug 27 '13 at 02:29

hash code HashMap

2 Answers2