Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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
Contributor

@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
@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
# for free to join this conversation on GitHub. Already have an account? # 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