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

Question: is the built-in hash function sufficient? #67

Open
njlr opened this issue Feb 26, 2018 · 1 comment
Open

Question: is the built-in hash function sufficient? #67

njlr opened this issue Feb 26, 2018 · 1 comment

Comments

@njlr
Copy link

njlr commented Feb 26, 2018

I was looking through the source and found that the hash function is quite simple:

import { stringHash } from '../common';

export function findHash (key: any): number {
  const hash: number = typeof key === 'number'
    ? key
    : typeof key === 'string'
      ? stringHash(key)
      : Math.abs(stringHash(JSON.stringify(key)));

  return hash;
}

I can see two problems that might occur with this.

First, JSON.stringify can be sensitive to property order, which might lead to unexpected behaviour:

> JSON.stringify({ x: 1, y: 2 })
'{"x":1,"y":2}'
> JSON.stringify({ y: 2, x: 1 })
'{"y":2,"x":1}'

Second, (and please correct me if I missed something!) it does not leave a possibility for customization. I think the set and map modules should be parameterized on a tuple of hash and equals. A default can be provided for convenience.

@axefrog
Copy link
Member

axefrog commented Mar 7, 2018

Hey, really sorry about the slow reply. I foolishly read the issue on my phone before going to sleep, and then forgot to follow up.

The HashMap collection was actually ported from another package (see the contributor credits), and the intent was to upgrade the hashing function there to use what's in the core library. It hasn't come up as an issue for me yet though, which is why it's remained on the backburner. I'm open to pull requests on this one. Note that what's in the core library should be ok in theory, but again if you see any issues there, I'm very much open to contributions that help me improve things. This project is very much alive, despite the apparent long periods of inactivity - I just have several projects I'm working on in tandem, and being just one person, I only have so much productive bandwidth to allocate at any given time.

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

No branches or pull requests

2 participants