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

fix: MapPosition.hashCode value distribution #1747

Merged
merged 1 commit into from
Dec 2, 2023
Merged

fix: MapPosition.hashCode value distribution #1747

merged 1 commit into from
Dec 2, 2023

Conversation

josxha
Copy link
Contributor

@josxha josxha commented Dec 2, 2023 •
edited
Loading

This PR adds a better value distribution of for MapPosition.hashCode.

Old

Adding the 3 hashCodes together has the effekt that relativly bigger values are more likely to happen. This increases the chance of hash code collision.

  @override
  int get hashCode => center.hashCode + bounds.hashCode + zoom.hashCode;

New

If we use .hashCode anyway let's do the following change:

  @override
  int get hashCode => Object.hash(center, bounds, zoom);

Can and should get cherry picked to v6.1.0-rc

@josxha josxha self-assigned this Dec 2, 2023
TesteurManiak
TesteurManiak approved these changes Dec 2, 2023
View reviewed changes
Copy link
Collaborator

@TesteurManiak TesteurManiak left a comment

Choose a reason for hiding this comment

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

LGTM, I'm wondering why it was like this in the first place 😅

@josxha josxha added the bug This issue reports broken functionality or another error label Dec 2, 2023
@JaffaKetchup JaffaKetchup changed the title fix: hashCode value distribution fix: MapPosition.hashCode value distribution Dec 2, 2023
@JaffaKetchup JaffaKetchup merged commit c0829b4 into fleaflet:master Dec 2, 2023
6 checks passed
@JaffaKetchup JaffaKetchup removed the bug This issue reports broken functionality or another error label Dec 2, 2023
josxha added a commit that referenced this pull request Dec 2, 2023
@josxha
fix: MapPosition.hashCode value distribution (#1747)
b64efaa
@josxha josxha added this to the v6.1 milestone Dec 2, 2023
josxha added a commit that referenced this pull request Dec 6, 2023
@josxha
Squashed commit of the following:
c4df50e 
commit ec81782
Author: Luka S <github@jaffaketchup.dev>
Date:   Sat Dec 2 20:48:08 2023 +0000

    chore: v6.1.0 release preparation (#1749)

commit df40d8c
Author: mootw <spencer@xqkz.net>
Date:   Sat Dec 2 10:49:29 2023 -0600

    perf: avoid caching a single multiplication (#1743)

commit ad8318b
Author: Joscha <34318751+josxha@users.noreply.github.com>
Date:   Sat Dec 2 16:39:21 2023 +0100

    refactor: example app plugins (#1744)

commit c0829b4
Author: Joscha <34318751+josxha@users.noreply.github.com>
Date:   Sat Dec 2 16:31:38 2023 +0100

    fix: `MapPosition.hashCode` value distribution (#1747)

commit 51d3eda
Author: Luka S <github@jaffaketchup.dev>
Date:   Sat Dec 2 11:34:13 2023 +0000

    revert: #1731 (#1745)

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

@TesteurManiak TesteurManiak TesteurManiak approved these changes

@JaffaKetchup JaffaKetchup JaffaKetchup approved these changes

@ibrierley ibrierley Awaiting requested review from ibrierley

@mootw mootw Awaiting requested review from mootw

Assignees

@josxha josxha

Labels
None yet
Projects
None yet
Milestone
v6.1
Development

Successfully merging this pull request may close these issues.
3 participants
@josxha @TesteurManiak @JaffaKetchup