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

Implement Eq and PartialEq for DashMap and DashSet #332

Merged
merged 1 commit into from
Mar 5, 2025

Conversation

noahbkim
Copy link
Contributor

@noahbkim noahbkim commented Mar 4, 2025

Hi, I was hoping to revisit #138, where you said

They aren't implemented because equality makes no sense for a concurrent map. Fundamentally you never really have to compare two and if you do, you're probably architecting your code in a non-optimal way.

I agree with this entirely in the context of real, running code, but am hoping to change your mind on the basis of testing. Providing equality methods makes writing test cases that reason about the contents of DashMap's and DashSet's much more concise, and testing is arguably the one scenario where ergonomics & readability clearly outweigh performance. Please consider giving users the option, even if you feel obligated to warn them against it, to use these traits.

Happy to make any tweaks and add documentation at your request. Thanks for all the work on this project!

@xacrimon
Copy link
Owner

xacrimon commented Mar 5, 2025

Hm, yes. You're right, this is an angle I didn't consider when I surely should have.

Thanks for bringing this to my attention, I will merge this now, place a TODO to add a sentence or two in docs about this for tomorrow. Cheers mate.

@xacrimon xacrimon merged commit 5cc2f90 into xacrimon:master Mar 5, 2025
6 checks passed
@noahbkim
Copy link
Contributor Author

noahbkim commented Mar 5, 2025

Legend, thanks!!

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