search

jasondavies / bloomfilter.js

JavaScript bloom filter using FNV for fast hashing
http://www.jasondavies.com/bloomfilter/
BSD 3-Clause "New" or "Revised" License
759 stars 79 forks source link

Fix post-processing step to match C code. #11

Closed rasky closed 9 years ago

rasky commented 10 years ago

The code needs to use an unsigned right-shift to match the original C code using unsigned integers. I don't know if this affects the quality of the output, but it can't hurt to match the original code.

jasondavies commented 9 years ago

Good find, but I don’t think the use of arithmetic vs logical right-shift affects the mixing negatively, so I’m keeping it in there for now.

rasky commented 9 years ago

I agree it doesn't, but it's a gratuitous deviation from the documented algorithm. It took me several hours of debugging to fix it because I had to create a bloomfilter in C++ that interoperates with your project and it's been complicated to do so because of this bug.

IOW, it's a difference that brings you nothing, and affects compatibility with other implementations, and you're one-click away from fixing it and forgetting about it :)

jasondavies commented 9 years ago

OK, you’ve convinced me, though I think there are probably other gotchas relating to string encodings if you really want compatibility between implementations (JavaScript uses UCS-2).