Skip to content

Commit

Permalink
Fix noisy hashCode/equals implementations for Family
Browse files Browse the repository at this point in the history
Signed-off-by: Anton Gustafsson <antag99@gmail.com>
  • Loading branch information
Anton Gustafsson committed May 15, 2015
1 parent db1286f commit b01cb15
Showing 1 changed file with 2 additions and 11 deletions.
13 changes: 2 additions & 11 deletions ashley/src/com/badlogic/ashley/core/Family.java
Original file line number Diff line number Diff line change
Expand Up @@ -160,21 +160,12 @@ public Family get () {

@Override
public int hashCode () {
final int prime = 31;
int result = 1;
result = prime * result + all.hashCode();
result = prime * result + one.hashCode();
result = prime * result + exclude.hashCode();
result = prime * result + index;
return result;
return index;
}

@Override
public boolean equals (Object obj) {
if (this == obj) return true;
if (!(obj instanceof Family)) return false;
Family other = (Family)obj;
return index == other.index && all.equals(other.all) && one.equals(other.one) && exclude.equals(other.exclude);
return this == obj;
}

private static String getFamilyHash (Bits all, Bits one, Bits exclude) {
Expand Down

5 comments on commit b01cb15

@ArtCraft
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since family isn't used as a key in the map, you can remove both this overrides at all

@antag99
Copy link
Contributor

@antag99 antag99 commented on b01cb15 Jun 8, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Family is used as key in Engine. The equals() implementation could indeed be removed, but that is not an improvement.

@dsaltares
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm inclined to say that equals() should compare indices for security reasons, although there should never be an instance of Family with different ref value and same index.

Wherever you override hashCode you should always override equals and viceversa.

@dsaltares
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ArtCraft could you make equals() compare indices? Theoretically, this class doesn't abide by the equals/hasCode contracts as you left it.

Are tests passing?

@antag99
Copy link
Contributor

@antag99 antag99 commented on b01cb15 Jun 9, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't break the contract, as hashCode() does not return different integers when equals() returns true.

Please sign in to comment.