Skip to content

Improve String-to-Number Conversion Logic in Seed Handling - #76

Open
allquantor wants to merge 1 commit into
andreasonny83:mainfrom
allquantor:fix/convert-numeric-seed
Open

Improve String-to-Number Conversion Logic in Seed Handling#76
allquantor wants to merge 1 commit into
andreasonny83:mainfrom
allquantor:fix/convert-numeric-seed

Conversation

@allquantor

@allquantor allquantor commented Oct 10, 2023

Copy link
Copy Markdown

This Pull Request introduces an optimized and potentially more collision-resistant string-to-number conversion method in the seed-handling logic.

Before:
With the current logic, different string seeds might result in the same numeric seed if the sum of their character codes is the same. For example:

"AB" would be converted to 138 because 'A' has a character code of 65, ' B' has a character code of 66, and 65 + 66 + 1 (the initial accumulator value) is 132.
"BA" would also be converted to 132 for the same reason.

Example:

const seed = "BA"; // Try changing this to "AB" and you'll get the same result
const numberFromString = seed.split('').reduce((acc, curr) => acc + curr.charCodeAt(0), 1);
const numericSeed = Math.floor(Number(numberFromString));
console.log(numericSeed); // Output: 132

Now:
A straightforward improvement to the hashing function enhances its capacity to disperse different input strings to a broader range of output values, thus reducing collisions.

@alekmeckaroski

alekmeckaroski commented Feb 7, 2024

Copy link
Copy Markdown

@andreasonny83
Is this PR merging?
I am using this library for generating names and I am using unique strings as seed.
Although I am working with large number of seeds, inevitably coming to a collision I want to make them minimal.
Please review the code and merge it so I can update.

Thanks

@andreasonny83

Copy link
Copy Markdown
Owner

@alekmeckaroski
Please make sure to update the tests and write new use cases for covering the change introduced

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.

3 participants