Utility classes do not override equals and hashCode

Started by mstWeal, August 14, 2010, 22:46:19

Previous topic - Next topic

mstWeal

Hi there,

I was just wondering why for example Vector3f does not override equals() and hashCode()? As I programmed the following code:

Vector3f position = entity.getPosition();
assertEquals(new Vector3f(0f, 0f, 0f), position);

This is not working without Vector3f overriding equals().

princec

Gadzooks! I wonder how nobody noticed this for so long. I imagine a patch should be forthcoming.

Cas :)

mstWeal

I think it cannot be changed anymore since it would break existing code. I've written my own class now. But maybe it would be good to introduce some new classes to LWJGL that may coexist with the old ones.

The classical Java way of designing such classes is making them immutable. This has several benefits, for example you can return the position vector of an entity directly without the need of defensively copying it because it cannot be changed from outside the entity anyways.

Here is the vector class that I've written now. It's not as fully featured as that of LWJGL because I've only included the things that I need.

public final class Vector3F {

	/**
	 * Returns a vector with the given values.
	 * 
	 * @param x
	 *            The x component of the vector.
	 * @param y
	 *            The y component of the vector.
	 * @param z
	 *            The z component of the vector.
	 */
	public static Vector3F valueOf(float x, float y, float z) {
		return new Vector3F(x, y, z);
	}

	/** Lazily initialized, cached hash code. */
	private volatile int hashCode;

	/** The x component of this vector. */
	private final float x;

	/** The y component of this vector. */
	private final float y;

	/** The z component of this vector. */
	private final float z;

	/**
	 * @param x
	 *            The x component of the vector.
	 * @param y
	 *            The y component of the vector.
	 * @param z
	 *            The z component of the vector.
	 */
	private Vector3F(float x, float y, float z) {
		this.x = x;
		this.y = y;
		this.z = z;
	}

	/**
	 * Returns the normalized version of this vector.
	 */
	public Vector3F normalize() {
		float lengthSquared = x * x + y * y + z * z;
		float length = (float) Math.sqrt(lengthSquared);
		return Vector3F.valueOf(x / length, y / length, z / length);
	}

	/**
	 * Returns a new vector that is the result of the cross product between this vector and the given right hand side
	 * vector.
	 * 
	 * @param right
	 *            The right hand side vector.
	 * 
	 * @throws NullPointerException
	 *             If <tt>vectorRight</tt> is <tt>null</tt>.
	 */
	public Vector3F cross(Vector3F right) {
		ArgumentCheck.notNull(right);
		float nx = y * right.z - z * right.y;
		float ny = right.x * z - right.z * x;
		float nz = x * right.y - y * right.x;
		return Vector3F.valueOf(nx, ny, nz);
	}

	/**
	 * Returns the x component of this vector.
	 */
	public float x() {
		return x;
	}

	/**
	 * Returns the y component of this vector.
	 */
	public float y() {
		return y;
	}

	/**
	 * Returns the z component of this vector.
	 */
	public float z() {
		return z;
	}

	@Override
	public boolean equals(Object obj) {
		if (obj == this) {
			return true;
		}
		if (!(obj instanceof Vector3F)) {
			return false;
		}
		Vector3F otherVector = (Vector3F) obj;
		return x == otherVector.x && y == otherVector.y && z == otherVector.z;
	}

	@Override
	public int hashCode() {
		int result = hashCode;
		if (result == 0) {
			result = 17;
			result = 31 * result + Float.floatToIntBits(x);
			result = 31 * result + Float.floatToIntBits(y);
			result = 31 * result + Float.floatToIntBits(z);
			hashCode = result;
		}
		return result;
	}

	@Override
	public String toString() {
		return "Vector3F [" + x + ", " + y + ", " + z + "]";
	}

}

mstWeal

Ok, coming back to this very shortly: Making these utility classes immutable is not a good idea at all. It drops performance by a lot as new objects would be created all the time with all the rotations and stuff going on. I never tought that it causes such a huge performance deficit but on my machine that's a difference of about 90 frames.

But still, hashCode() and equals() should be overridden.

Cheers, Alex