Hashtable documentation #93
Conversation
| #[derive(Debug, PartialEq, Eq)] | ||
| struct TestKey(usize); | ||
|
|
||
| impl Hashable for TestKey { | ||
| fn hash(&self) -> usize { | ||
| self.0 | ||
| } | ||
| } |
There was a problem hiding this comment.
Ah, so this is valuable, because this way we test that the hash table works with hashable elements.
What's interesting here is that no tests broke, which means that we aren't testing the actual placement of keys in the table (Testkey(i) would be placed in the i-th bucket, but i, would be placed in the hash(i) % length bucket).
I think this is a bit involved, and I'm not sure how educational it is, so I think it's fine to remove this. wdyt?
There was a problem hiding this comment.
I agree. What was actually upsetting about this was that the key educational bit of a hash table, the bit that makes it 'different' than other data structures, is the hash function...which we are just setting it to the item! which means it might as well just be an array.
But wait! There is more!
We are using a vector and a linked list internally! Which is...just. no.
We definitely should have a linked list as a structure as an example, but we shouldn't be using them internally. The cache thrashing they cause is just not worth their use in modern code. The cost vs benefit just fails. Arena's are significantly better choices and that goes doubly so given rust's hate of multi-borrow mechanics.
vec of Option for storage, K is the index into the vec, v is the value in the vec, and we are off and running. The same could be done with the b-tree but I just couldn't be bothered since I've been focusing on the doc's instead of the core implementation.
There was a problem hiding this comment.
If you'd like to implement that, I'm more than happy to review it!
I still think it's valuable to show suboptimal implementations though: if someone is starting, they should have access to both the dumb, easy implementation and the nicer, optimized, linear-probing implementation.
There was a problem hiding this comment.
I'm merging this, feel free to open a separate PR!
No description provided.