Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pr refactors the hashing function so we do not keep global state, it also implements a new
Hash.makeSeeded
andHash.make
function which returns aHashInstance
. If we want a different api I am happy to change things around.API:
Questions:
Hash.make
every time or do we want to use the globalSeed? Maybe if we want a globalSeed we should provide it amakeGlobal
.makeSeeded
with a set seed onMap
andSet
so we can get rid of the possible exception.My original implementation had the signature of
Hash.make: (seed: Number) => (anything: a) => Number
but this causes our inference engine to lock the hashing function a particular data type when you do something like:The alternative approach suggested was to use a submodule for seeded but as the api's would differ this did not seem ideal.
One benefit to this approach is because we call
Random.random()
onmake
we will not throw a possible exception onHash.hash
which means all of ourSet
andMap
functions besidesmake
wont throw.Succeeds: #1952
This is breaking as we changed the type of
Hash.hash
from(a) => Number
to(HashInstance, a) => Number
.