Skip to content

Lang 1696 equals builder collections equals delegate#1615

Closed
ShreyanshRaj22 wants to merge 2 commits intoapache:masterfrom
ShreyanshRaj22:LANG-1696-equals-builder-collections-equals-delegate
Closed

Lang 1696 equals builder collections equals delegate#1615
ShreyanshRaj22 wants to merge 2 commits intoapache:masterfrom
ShreyanshRaj22:LANG-1696-equals-builder-collections-equals-delegate

Conversation

@ShreyanshRaj22
Copy link

@ShreyanshRaj22 ShreyanshRaj22 commented Mar 7, 2026

Description

This PR addresses LANG-1696 by improving the behavior of EqualsBuilder.reflectionEquals when comparing Collection and Map instances. https://issues.apache.org/jira/browse/LANG-1696

Instead of performing reflective field comparison on these types, the implementation now delegates directly to their equals methods. Since Collection and Map already define well-established equality semantics based on their contents, this produces more consistent and expected results when comparing different implementations containing the same elements.

Changes

  • Detect Collection and Map instances in reflectionEquals and delegate comparison to their equals implementations.
  • Added a unit test verifying equality between different collection implementations (e.g., HashSet and Collections.emptySet()).

Compatibility

As noted in LANG-1696, this may change equality outcomes in cases where reflective comparison previously produced different results, but the new behavior aligns with the standard semantics of the Java Collections Framework.

Thanks for your contribution to Apache Commons! Your help is appreciated!

Before you push a pull request, review this list:

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

@garydgregory
Copy link
Member

garydgregory commented Mar 7, 2026

-1 for now:

This may break existing applications that rely on reflection, since they asked for reflection to be used in the first place. To see why, rebase on git master and run EqualsBuilderTest.

I don't think we should start down the road of having special cases for this or that class, or it'll never end.

I can also imagine an implementation of a collection or map that doesn't correctly implement equals(), breaking.

I think it is safer for now to leave this reflection feature alone.

@ShreyanshRaj22
Copy link
Author

Thanks for the feedback. I see how this change could break existing behavior relying on reflection comparison. I briefly considered implementing this as an optional flag-based toggle (as mentioned in the ticket), something like EqualsBuilder(obj1, obj2, shouldUseDefaultEquals), but that would likely introduce additional complexity for a fairly narrow use case. Given the backward compatibility concerns, I'll go ahead and close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants