Java ArrayList indexOf returns -1

Question

I have a strange problem with my code.

Heres the code I test the Chunk class with:

List<Chunk> chunks = new ArrayList<Chunk>();
chunks.add(new Chunk(1,1,1));
System.out.println(chunks.indexOf(new Vector3i(1, 1, 1)));

And here is the Chunk class' equals method:

public boolean equals(Object object) {
    System.out.println("Test _1_");
    if (object != null && object instanceof Vector3i) {
        System.out.println("Test _2_");
        if((this.x == ((Vector3i) object).x)&&(this.y == ((Vector3i) object).y)&&(this.z == ((Vector3i) object).z)) {
            System.out.println("Test _3_");
            return true;
        }
    }
    System.out.println("Test _4_");

    return false;
}

The Vector3i:

public class Vector3i {
    public int x;
    public int y;
    public int z;


    public Vector3i(int x, int y, int z) {
        this.x = x;
        this.y = y;
        this.z = z;
    }

}

When I run it, it just returns -1. The test prints from the equals method doesn't print, which means that it's not even begin executed. Why is that?

This is a serious abuse of the [`Object.equals()` contract](http://docs.oracle.com/javase/8/docs/api/java/lang/Object.html#equals-java.lang.Object-). Your `equals()` method isn't **reflexive**, **symmetric** or **transitive**. Use a `Map` instead to associate a unique chunk with a given vector value. — biziclop, Jun 15 '15 at 11:51
You have a class named `Chunk` with an `equals` method that checks if the argument is some other type? — MadConan, Jun 15 '15 at 11:54
@MadConan It's because chunk contains lots of data, and it would be too heavy if I should compare it with a new one. — KaareZ, Jun 15 '15 at 12:00

Tagir Valeev · Accepted Answer · 2015-06-15T12:23:49.203

5

If you check the ArrayList.indexOf implementation, you will see that Vector3i.equals is called in your case. Actually it's even specified in JavaDoc for List:

More formally, returns the lowest index i such that (o==null ? get(i)==null : o.equals(get(i))), or -1 if there is no such index.

In general equals operation must be symmetric: a.equals(b) == b.equals(a). So you have to implement Vector3i.equals as well.

Please note also that your current equals implementation lacks other properties like reflexivity. Also consider implementing the hashCode when you implement an equals.

edited Jun 15 '15 at 12:23

answered Jun 15 '15 at 11:50

Tagir Valeev

97,161
19
222
334

Thank you! This made it. – KaareZ Jun 15 '15 at 11:53
1

While this is 100% true, it is also terrible advice. For a start, you've just broken the `Object.hashCode()` contract, so I'm expecting a "Why does my `HashMap.get()` return `null`?" question soon. Secondly, it still doesn't make `equals()` reflexive. – biziclop Jun 15 '15 at 12:05
1

@biziclop, well, when I say "implement equals", it's implied that it's a good idea to implement the `hashCode` as well. As for other `equals()` properties (reflexivity, transitivity, consistency), it's well written in the linked JavaDoc. Well, I'll add a notice to the answer. – Tagir Valeev Jun 15 '15 at 12:22

score 0 · Answer 2 · answered Jun 15 '15 at 11:51

0

chunks.indexOf(new Vector3i(1, 1, 1)

calls equals() method on Vector3i class, howeve, not on Chunk class...

answered Jun 15 '15 at 11:51

Pavan Kumar K

1,360
9
11

Java ArrayList indexOf returns -1

2 Answers2