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

discussion: should hashcode be calculated based on order #2742

Closed
wants to merge 1 commit into from

Conversation

jarlah
Copy link

@jarlah jarlah commented Apr 26, 2023

What concerns me is that changing hashCode in such a big way can affect other parts of vavr, like equals. However, it seemed to not make any difference to comparing the same two (different) hash maps I made a test for with assertThat(...).isNotEqualTo(...), which is also the example from #2733

In addition, this Collections.hasOrdered and Collections.hashUnordered is used many other places. Should we change them too?

Look at this as a discussion PR, more than actual fix.

DONTMERGE :) (unless it was an ingenious fix, which I doubt)

PS! is this project used actively still? to be honest, I would rather use vavr than mock around with streams and collect and wierd reduce mechanics.

@jarlah jarlah requested a review from danieldietrich as a code owner April 26, 2023 13:20
@jarlah jarlah changed the title fix: hashcode should be calculated based on order discussion: should hashcode be calculated based on order Apr 26, 2023
@pivovarit
Copy link
Member

LinkedHashMap is still just a HashMap with an insertion history, and that's also how JDK treats it:

var m1 = new LinkedHashMap<String, Integer>();
var m2 = new LinkedHashMap<String, Integer>();

m1.put("a", 1);
m1.put("b", 1);
m2.put("b", 1);
m2.put("a", 1);

System.out.println("m1.equals(m2) = " + m1.equals(m2));
System.out.println("m1.hashCode() = " + m1.hashCode());
System.out.println("m2.hashCode() = " + m2.hashCode());

so a natural course of action is to stay consistent with this approach and use ordered hashing for data structures that rely on the ordering of elements

@pivovarit pivovarit closed this Aug 21, 2024
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