Assert bounds in shift_insert and add insert_before
#340
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The only difference compared to using `shift_insert` is when the binary-searched key isn't *actually* new to the map, just not found in properly sorted order. In this case we can't guarantee a sorted result either, but it will at least behave better about the new position, especially if that's the end.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The way
shift_insertworks is a little weird around existing entries, because it's not possible to move them toindex == len()-- that only works when inserting new entries. This caveat is now documented and directly asserted, so the panic will be clearer than the deeper bounds-check panic we were throwing before.As an alternative, I've also added
insert_beforewhich is looser about the exact index when moving entries. There's no difference for new entries, but existing entries might move toindex - 1if that keeps them right before the entry at the given index, or the end point. In this case, all indices in0..=len()are always valid choices.That new semantic is also a little better for the implementation of
insert_sorted, although that only matters in the case where binary search didn't find a key, but it does exist in the map. That means that the keys are not properly sorted to begin with, but at leastinsert_beforelets us try to deal with that without panicking.