Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add hash code override methods where equals method is overwritten #100

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

add hash code override methods where equals method is overwritten #100

wants to merge 7 commits into from

Conversation

elliVM
Copy link
Contributor

@elliVM elliVM commented Oct 14, 2024 •
edited
Loading

Use Objects.hash(values) to generate hash code for objects where equals() method is overwritten.

Objects.hash uses formula result = 31 * result + elementHashCode for good hash distribution.
31 * result which is same as (x << 5) - result) (bitwise shift) is optimized for JVM on modern hardware.

null safe method, null values return hash value of 0.

note: fixed missing object parameter value in ConditionConfig equals method and added test for class

@elliVM elliVM self-assigned this Oct 14, 2024
kortemik
kortemik requested changes Oct 14, 2024
View reviewed changes
Copy link
Member

@kortemik kortemik left a comment

Choose a reason for hiding this comment

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

see https://www.baeldung.com/java-equals-hashcode-contracts

use equalsverifier in tests

src/main/java/com/teragrep/pth_06/planner/CategoryTableImpl.java Outdated
@@ -175,4 +177,9 @@ public boolean equals(final Object object) {
return this.originTable.equals(cast.originTable) && this.ctx == cast.ctx && // equal only if same instance of DSLContext
Copy link
Member

Choose a reason for hiding this comment

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

table condition missing from equals

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added missing field

src/main/java/com/teragrep/pth_06/planner/TableFilters.java Outdated
@@ -165,4 +166,9 @@ public boolean equals(final Object object) {
return this.ctx == cast.ctx && this.value.equals(cast.value) && this.table.equals(cast.table)
Copy link
Member

Choose a reason for hiding this comment

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

recordsInMetadata missing from equals

src/main/java/com/teragrep/pth_06/planner/walker/conditions/IndexStatementCondition.java Outdated
@@ -136,4 +137,9 @@ public boolean equals(final Object object) {
final IndexStatementCondition cast = (IndexStatementCondition) object;
return this.value.equals(cast.value) && this.config.equals(cast.config);
Copy link
Member

Choose a reason for hiding this comment

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

missing fields in equals

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added missing field

src/main/java/com/teragrep/pth_06/planner/walker/conditions/LatestCondition.java Outdated

@Override
public int hashCode() {
return Objects.hashCode(value);
Copy link
Member

Choose a reason for hiding this comment

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

wrong method used Objects.hashCode(value) while should use Objects.hash(value)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to Objects.hash(value)

src/main/java/com/teragrep/pth_06/planner/walker/conditions/PatternMatchCondition.java Outdated

@Override
public int hashCode() {
return Objects.hashCode(value);
Copy link
Member

Choose a reason for hiding this comment

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

wrong method used Objects.hashCode(value) while should use Objects.hash(value)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to Objects.hash(value)

src/main/java/com/teragrep/pth_06/planner/walker/conditions/ValidElement.java Outdated

@Override
public int hashCode() {
return Objects.hashCode(element);
Copy link
Member

Choose a reason for hiding this comment

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

wrong method used Objects.hashCode(element) while should use Objects.hash(element)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed to Objects.hash(value)

src/main/java/com/teragrep/pth_06/planner/walker/conditions/ValidElement.java Outdated
@@ -100,4 +102,9 @@ public boolean equals(final Object object) {
boolean equalValue = this.element.getAttribute("value").equals(cast.element.getAttribute("value"));
return equalName && equalOperation && equalValue;
Copy link
Member

Choose a reason for hiding this comment

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

equal() and hashCode calculation methods differ

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed methods to use only element field

@elliVM elliVM marked this pull request as draft October 14, 2024 12:39
@elliVM elliVM removed the request for review from StrongestNumber9 October 14, 2024 12:40
@elliVM elliVM marked this pull request as ready for review October 16, 2024 08:13
@elliVM
Copy link
Contributor Author

elliVM commented Oct 16, 2024

added equalsverifier libary tests

51-code
51-code requested changes Oct 17, 2024
View reviewed changes
src/test/java/com/teragrep/pth_06/config/ConditionConfigTest.java
DSLContext ctx = DSL.using(new MockConnection(c -> new MockResult[0]));

@Test
void testEquality() {
Copy link

Choose a reason for hiding this comment

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

public keyword missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made test classes and methods public

Copy link

Choose a reason for hiding this comment

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

Still missing from ConditionConfigTest?

src/test/java/com/teragrep/pth_06/planner/TokenizedValueTest.java Outdated
@@ -86,4 +87,18 @@ void testNotEquals() {
Assertions.assertNotEquals(value2, value1);
Assertions.assertNotEquals(value1, null);
}

@Test
void testHashCode() {
Copy link

Choose a reason for hiding this comment

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

public keyword missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

made test classes and methods public

@elliVM elliVM requested a review from 51-code October 18, 2024 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kortemik kortemik kortemik requested changes

@51-code 51-code Awaiting requested review from 51-code

Requested changes must be addressed to merge this pull request.

Assignees

@elliVM elliVM

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@elliVM @kortemik @51-code