Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose Iterator types #56

Closed
jakoschiko opened this issue Aug 4, 2024 · 5 comments · Fixed by #72
Closed

Expose Iterator types #56

jakoschiko opened this issue Aug 4, 2024 · 5 comments · Fixed by #72

Comments

@jakoschiko
Copy link
Contributor

The Iterator types of intmap are currently private, see the doc. Based on the source code I'm not sure if this was an intentional decision.

As long as rust-lang/rust#63063 is not merged, it's important to expose these type so that they can be used inside of structs and enums.

I suggest:

  • Expose the current Iterator types by re-exporting them at top-level
  • Remove the type parameter K from the current Iterator types. It feels a little bit inconsistent because IntMap don't have this type parameter either.
@JesperAxelsson
Copy link
Owner

JesperAxelsson commented Aug 9, 2024

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.

@jakoschiko
Copy link
Contributor Author

I also thought about supporting other integer types. Any reason you dropped that idea?

@JesperAxelsson
Copy link
Owner

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.

@jakoschiko
Copy link
Contributor Author

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.

@JesperAxelsson
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants