Expand removal options

[cuviper]

This aims to make the performance tradeoffs explicit when removing items.

• New shift_remove and shift_take perform O(n) removal, like Vec::remove
• The existing swap_* methods already perform like Vec::swap_remove
• Ambiguous remove and take methods, which currently swap, are deprecated in favor of explicit shift/swap methods.

Fixes #90.

[cuviper]

As we're talking about a new release, I realized that I was also sitting on these changes in a branch...

[bluss]

Makes sense. The release is already pushed out, but there can be more releases 🙂

What kind of deprecation is intended? My first thought that it would be a "permanently" deprecated method, since one design guideline was drop-in compatibility with HashMap. So you can switch hashmap implementations easily, and with the warning you can realize you have a choice of removal option.

I'm afraid the deprecation message will be annoying for those that don't mind the current behaviour?

[bluss]

looks good to me (and adding shift removal is a good idea!)

[cuviper]

What kind of deprecation is intended? My first thought that it would be a "permanently" deprecated method, since one design guideline was drop-in compatibility with HashMap. So you can switch hashmap implementations easily, and with the warning you can realize you have a choice of removal option.

Yes, this makes sense to me. We have a similar situation on rayon find with a deprecation note. I think later we added the #[doc(hidden)] to avoid clutter.

I'm afraid the deprecation message will be annoying for those that don't mind the current behaviour?

Maybe so, but it's an easy fix to use swap_ or #[allow(deprecated)]. Do you envision folks wanting to flip back and forth? I'd think that would only be during some initial evaluation.

[bluss]

Thank you for adding this! tests/ directory has no mention of shift_remove 😅 and I'm adding a quickcheck test now for that reason.

[bluss]

Indexmap 1.2 is released including this feature 🙂