Question

I have the following immutable HSL class that I use to represent a colour and aide in calculations on RGBA colours with a little more finesse.

public class HSL {
  protected final float hue;
  protected final float saturation;
  protected final float lightness;

  public HSL(float hue, float saturation, float lightness) {
    this.hue = hue;
    this.saturation = saturation;
    this.lightness = lightness;
  }

  // [snip] Removed some calculation helper functions

  @Override
  public String toString() {
    return "HSL(" + hue + ", " + saturation + ", " + lightness + ")";
  }

  @Override
  public int hashCode() {
    return
        37 * Float.floatToIntBits(hue) +
        37 * Float.floatToIntBits(saturation) +
        37 * Float.floatToIntBits(lightness);
  }

  @Override
  public boolean equals(Object o) {
    if (o == null || !(o instanceof HSL)) {
      return false;
    }

    HSL hsl = (HSL)o;
    // We're only worried about 4 decimal places of accuracy. That's more than the 24b RGB space can represent anyway.
    return
        Math.abs(this.hue - hsl.hue) < 0.0001f &&
        Math.abs(this.lightness - hsl.lightness) < 0.0001f &&
        Math.abs(this.saturation - hsl.saturation) < 0.0001f;
  }
}

While I don't foresee using this particular class in a HashMap or similar as it's an intermediary between a similar RGBA class which uses ints for it's internal storage, I'm somewhat concerned about the general case of using floats in equality. You can only make the epsilon for comparison so small, and even if you could make it arbitrarily small there are many float values that can be represented internally in multiple ways leading to different values returned from Float.floatToIntBits. What is the best way to get around all this? Or is it not an issue in reality and I'm just overthinking things?

Était-ce utile?

La solution

The other answers do a great job of explaining why your current design may cause problems. Rather than repeat that, I'll propose a solution.

If you care about the accuracy only to a certain number of decimal places (it appears 4), you could multiply all incoming float values by 10,000 and manage them as long values. Then your equality and hashcode calculations are exact.

I assume you've omitted the getters from your class for brevity. If so, ensure your Javadocs clearly explain the loss of precision that will occur when passing a float into your class constructor and retrieving it from a getter.

Autres conseils

This definition of equals() is not transitive. It really shouldn't be called equals(). This and the hashCode() issue would most likely silently bite when used in the Collections API. Things like HashSet would not work as expected and methods like remove(). For purposes here you should test for exact equality.

I think you are correct to be concerned about the general case of hashCode() diverging heavily from equals().

The violation of the general convention that hashes of two "equal" objects should have the same hashCode() will most likely lead to all sorts of unexpected behavior if this object is used in the future.

For starters, any library code that makes the usual assumption that unequal hashCodes implies unequal objects will find a lot of unequal objects where it should have found equal objects, because the hashCode check usually comes first, for performance.

hashcode is like defining a bucket that an object should reside in. With hashing, if an object is not in its right bucket, you cannot find that with the equals() method.

Therefore, all the equal objects must reside in the same bucket (i.e., their hashcode() method should return the same result) or the results would be unpredictable.

I think you could try to weaken the hashCode implementation to prevent it from violating contract with equals - I mean ensure that when equals returns true, then hashCode returns the same value but not necessarily the other way around.

Licencié sous: CC-BY-SA avec attribution
Non affilié à StackOverflow
scroll top