Make BFHash take on non negative values only, issue #229 #243
Conversation
… added scalacheck that the new indices are non negative and a test for the problematic case
|
should remove seed variable to align with PR #220 (in the tests) |
| } | ||
|
|
||
| val negativeBFHash = NegativeBFHash(NUM_HASHES, WIDTH, SEED) | ||
| val bfHash = BFHash(NUM_HASHES, WIDTH, SEED) |
There was a problem hiding this comment.
Use scalacheck to generate these rather than being fixed magic values
There was a problem hiding this comment.
Well, ALSO have fixed magic values. These were found to have the bug, so let's keep them to prevent regression.
There was a problem hiding this comment.
Well for the Specification section rather than the scalacheck one we should try generate the exact problem case, not sure it would make any benefit here.
There was a problem hiding this comment.
There is a test for the problem case in the Specification already.
I'll make the property generic with an arbitrary "negative" hash.
|
Great to see a pull request on this, thanks! Just a few small things |
Make BFHash take on non negative values only, issue #229
No description provided.