Open jakoschiko opened 1 month ago
Yeah, the Iter
structs where in lib.rs before so should have been exported then. I'm fine with exporting them.
About the K
generic it was more a case of my type fu failing me. I also had some thoughts of supporting different keys sizes like u32
and u16
.
I also thought about supporting other integer types. Any reason you dropped that idea?
Mostly ran out of motivation. And back when I started writing this project const compilation weren't ready yet. Changing the constant used during hashing should be enough to support different int types. I thought about being able to select growth algo. Now IntMap use power of two which is quite easy and fast but can be wasteful. Most other hashmaps use increase in switch statement with fixed primes. This might be possible to do the same way as the std hashmap injects hashers. The biggest downside I see is increased complexity. IntMap is pretty readable right now. The more you introduce generics and type level programming the more complicated it becomes. The standard hashmap can be a bit tricky to read.
Not sure about the growth algo, but support for different integer types would be very nice in my opinion. And I'm not convinced yet that this would increase complexity significantly. I think it should even be possible without breaking changes thanks to default type parameters, e.g. IntMap<V, I: Int = u64>
. If you don't mind, I will try it out.
The growth algorithm should be fine, it only changes the number of elements in the backing vector. You would need to be able to switch between hash_u64
and hash_uXX
during compile time. The magic number should be a large prime, preferably the largest of each uint size.
The
Iterator
types ofintmap
are currently private, see the doc. Based on the source code I'm not sure if this was an intentional decision.As long as https://github.com/rust-lang/rust/issues/63063 is not merged, it's important to expose these type so that they can be used inside of structs and enums.
I suggest:
Iterator
types by re-exporting them at top-levelK
from the currentIterator
types. It feels a little bit inconsistent becauseIntMap
don't have this type parameter either.