get function takes a pointer to usize?

[Binero]

Why does the get function (amongst others) take a pointer to usize, as opposed to just a usize?

https://github.com/contain-rs/vec-map/blob/master/src/lib.rs#L433
https://github.com/contain-rs/vec-map/blob/master/src/lib.rs#L457
https://github.com/contain-rs/vec-map/blob/master/src/lib.rs#L475
https://github.com/contain-rs/vec-map/blob/master/src/lib.rs#L475

[apasel422]

The reason is essentially consistency with the other map types (e.g. BTreeMap).

CC contain-rs/bit-set#5.

[Binero]

Seems like a waste of resources to me.

[apasel422]

With inlining and other optimizations, I'd say the main downside to the current situation is syntactic noise. I'd be in favor of merging a pull request that changes these to take the usize by value.

[Binero]

Such a change would be a breaking change. If I were to write a PR, would it be approved?

[apasel422]

Other people from @contain-rs/owners can chime in on whether such a minor change is worth breaking backwards compatibility, but if we do merge such a PR, we can bump the crate's version to indicate that there was a breaking change.

[Binero]

cc @reem

[Gankra]

We have 12 reverse dependencies https://crates.io/crates/vec_map/reverse_dependencies and all of them are presumably going to break if we make this change. That seems way too brutal for something that I'd be surprised to not see inlined away.

CC @HeroesGrave @simnalamburt @andelf @johanneskoester

[johanneskoester]

In my opinion, it would be better to get the API right at this early stage of rust instead of breaking later when hundreds of packages depend on this.

[Binero]

I would have to agree with this.

[HeroesGrave]

I'm fine with it being changed. In all the cases where I used it I was calling vec_map.get[_mut](&index) anyway.

Assuming all of the downstream crates aren't using wildcard dependencies, it really shouldn't be that much of an issue anyway.

[Binero]

Well looking at what happened when libc updated recently, I wouldn't make that assumption.

[Gankra]

Upon reflection, if a trait is ever added, it can trivially dispatch to the proposed interface. So 👍 let's ship it.

[Binero]

Do we have a consensus here that the get function should take a value?

[apasel422]

@Binero I think so. You can submit a PR if you'd like.

[apasel422]

This was closed in #11.