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.

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

Remove unnecessary equals and hashcode tests #34

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

51-code
Copy link
Contributor

@51-code 51-code commented Feb 10, 2025

Fixes #33 .

Coverity flagged an equality test in ImmutableMapTest.java because of an unnecessary method call. Removing just that line makes the equality test quite unnecessary itself as the function call was there to check for side-effects.

My thought process here:

  1. Remove the line of code.
  2. Remove the whole test as it's useless now. There is equalsverifier tests anyway!
  3. Why not remove all manual equals tests to remove clutter, equalsverifier tool is used anyway.

Added bonus:
Coverity won't flag other tests in the future, because many other equality tests had similar unnecessary function calls but for some reason they were not flagged yet.

TLDR:
Equalsverifier is used in the project, so manual equality tests are not necessary. Coverity had flagged one of the manual tests, might flag more later.

@51-code 51-code added the bug Something isn't working label Feb 10, 2025
@51-code 51-code requested a review from Tiihott February 10, 2025 08:14
@51-code 51-code self-assigned this Feb 10, 2025
Copy link

@Tiihott Tiihott left a comment

Choose a reason for hiding this comment

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

As EqualsVerifier tests the equals() and hashCode() automatically with a single line, the manual tests become redundant and they should be fine to remove when the EqualsVerifier tests are implemented properly.
LGTM

@51-code 51-code requested a review from kortemik February 11, 2025 08:33
@kortemik kortemik merged commit a1395a1 into teragrep:main Feb 12, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove useless function calls
3 participants