Collection.contains yields wrong result

Question

I've got an object which has got a couple of fields -- as you can see the hashcode and equals method is implemented just taking the id in account:

public class SpotResponse{
String id;
// bla bla other fields
public SpotResponse() {
}

public SpotResponse(@NonNull String id) {
    this.id = id;
}
public String getId() {
    return id;
}

@Override
public boolean equals(Object o) {
    if (this == o) return true;
    if (o == null || getClass() != o.getClass()) return false;

    SpotResponse that = (SpotResponse) o;

    return id == that.id;
}

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

I've got a method which checks if a Collection<SpotResponse> newSpots

contains some oldSpots from a HashMap<String, SpotResponse> spots = new HashMap<>();

If I do this:

List<String> newKeys = new ArrayList<>();

    for (SpotResponse response : newSpots) {
        newKeys.add(response.getId());
    }

    for (SpotResponse oldSpot : spots.values()) {
        if (newKeys.contains(oldSpot.getId())) {
            continue;
        }
        /* blabla */
    }

newKeys.contains() returns true properly, but if instead I do

newSpots.contains(oldSpot)

It always returns false. In this case, the Collection is an ArrayList (if this is of any help)

That's because you're not adding anything to newSpots. You're adding to newKeys — Kwright02, May 09 '18 at 22:01
@Kwright02 newSpots already contains some payload, I'm just not comparing the String id's and relying on the comparisons between the actual SpotResponse objects — ntakouris, May 09 '18 at 22:03
Possible duplicate of [How do I compare strings in Java?](https://stackoverflow.com/questions/513832/how-do-i-compare-strings-in-java) — Johannes Kuhn, May 09 '18 at 22:11
Also, don't compare objects using `other.getClass() == getClass()`. There might be some reason (e.g. JPA) to subclass this, when it should be still considered equal. Use `instanceof` instead. — Johannes Kuhn, May 09 '18 at 22:13

score 2 · Accepted Answer · answered May 09 '18 at 22:11

2

Your bug is in your equals implementation, on this line:

return id == that.id;

You're comparing two Strings (namely id and that.id) with ==, when you should use id.equals(that.id).

answered May 09 '18 at 22:11

k_ssb

6,024
23
47

Sam · Answer 2 · 2018-05-09T22:22:35.313

0

You have to be careful when using Contains with Primitives. String is technically an object class wrapping a primitive of chars, but when you are comparing it, it is not comparing the literal object memory pointer, it is comparing the value at the memory pointer.

Contains is using .equals under the hood, so when overriding equals in your class you can't default back to the == comparison as that compares address and not necessarily value.

Hope that helps.

edited May 09 '18 at 22:22

answered May 09 '18 at 22:05

Sam

5,342
1
23
39

"compareTo("String") == 0" what's wrong with `equals`? – Andy Turner May 09 '18 at 22:09
This answer is wrong. `contains` correctly checks for object equality using the object's `equals` method. It is **not** just comparing memory pointers. – k_ssb May 09 '18 at 22:11
compares can have null point exceptions if one side is null, equals will not. Also equals does an "instance Of" call which takes slightly more performance – Sam May 09 '18 at 22:13
@Sam That's completely besides the point. Your statement "contains is comparing memory pointers like does this array contain this memory pointer" is wrong. `contains` checks for the existence of an element that is `equal` to the searched object. – k_ssb May 09 '18 at 22:14

Collection.contains yields wrong result

2 Answers2