Fix hash code collision for Vector2D and Vector3D

[ivanshuba]

This commit modifies the hashCode() method for Vector2D and the Vector3D classes in order to ensure unique values are generated for non-identical instances of vectors.

The proposed approach for calculating the hash code value is the same that is used in JTS and HE_Mesh projects. And as quoted in JTS project, this approach was initially 'borrowed' from the Joshua Bloch book 'Effective Java, 1st ed, 2008 ( the 2nd edition doesn't contain this recipe ).

In the "Item 9: Always override hashCode when you override equals" he describes the recipe as follows:

1. Store some constant nonzero value, say 17, in an int variable called result.
2. For each significant field f in your object, do the following: a. Compute an int hash code c for the field: i ... ii ... iii If the field is a long, compute (int)(f^(f>>>32)) iv ... v If the field is a double, compute Double.doublToLongBits(f), and then hash the resulting long as in step 2.a.iii vi ... vii ... b. Combine the hash code c computed in step 2.a into result as follows: result = 31 * result + c

[aherbert]

I would use int Double.hashCode(double) which effectively does the same thing but does not require as many statements, e.g.

    public int hashCode() {
        if (isNaN()) {
            return 642;
        }
        int result = Double.hashCode(x);
        result = 31 * result + Double.hashCode(y);
        result = 31 * result + Double.hashCode(z);
        return result;
    }

[ivanshuba]

@aherbert You're correct this is more readable while keeping the overall approach the same. I've checked, and all tests are passing.

[ivanshuba]

I've checked, and all tests are passing.

@aherbert Well, not all apparently. Only the ones I've added.

I'll try to find out what causes the GreatCircleTest to fail.

[ivanshuba]

@aherbert

GreatCircle class was using the Objects.hash() wrapper method to calculate hash value. In its turn, it was calling the Arrays.hashCode() method.

Unfortunately, somewhere during the process, this fails to prevent hash collisions when it's calculated for the GreatCircle instances. I haven't investigated the cause yet. As a guess, this might be related to that the Precision.DoubleEquivalence's hashCode() method is not overridden, not sure.

Anyways, the usage of the same approach as in Vector2D and Vector3D classes resolves the issue for the GreatCircle also.

@Override
public int hashCode() {
    int result = 1;
    long temp;
    temp = pole.hashCode();
    result = 31 * result + (int) (temp ^ (temp >>> 32));
    temp = u.hashCode();
    result = 31 * result + (int) (temp ^ (temp >>> 32));
    temp = v.hashCode();
    result = 31 * result + (int) (temp ^ (temp >>> 32));
    temp = getPrecision().hashCode();
    result = 31 * result + (int) (temp ^ (temp >>> 32));
    return result;
}

Additionally, I fixed my incorrect indentations and variable naming added previously to prevent check errors.

All tests are passing now when I run them through Maven.

[ivanshuba]

Hi @aherbert

Can I ask if you want me to fix anything to get this moving on?

[singhbaljit]

A little late to the party, but I think we should consider using https://github.com/jqno/equalsverifier to test hashCode and equals in a standard way, instead of writing all different permutations in the tests.

[darkma773r]

Even later to the party here... :-)

@singhbaljit, I would be on board with trying out EqualsVerifier. We already have GEOMETRY-107 for this.

@aherbert, @ivanshuba, we are still waiting on the requested changes, correct?

[aherbert]

The changes I requested to the tests have not been implemented. Without them the tests could sporadically fail.

[aherbert]

These hashcode changes have been integrated into the master branch.