Skip to content

Commit

Permalink
Use insert_before for "new" entries in insert_sorted
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
cuviper committed Aug 30, 2024
1 parent 7224def commit 8ca01b0
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 3 deletions.
4 changes: 2 additions & 2 deletions src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ where
///
/// This is equivalent to finding the position with
/// [`binary_search_keys`][Self::binary_search_keys], then either updating
/// it or calling [`shift_insert`][Self::shift_insert] for a new key.
/// it or calling [`insert_before`][Self::insert_before] for a new key.
///
/// If the sorted key is found in the map, its corresponding value is
/// updated with `value`, and the older value is returned inside
Expand All @@ -441,7 +441,7 @@ where
{
match self.binary_search_keys(&key) {
Ok(i) => (i, Some(mem::replace(&mut self[i], value))),
Err(i) => (i, self.shift_insert(i, key, value)),
Err(i) => self.insert_before(i, key, value),
}
}

Expand Down
28 changes: 28 additions & 0 deletions src/map/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,34 @@ fn shift_insert() {
}
}

#[test]
fn insert_sorted_bad() {
let mut map = IndexMap::new();
map.insert(10, ());
for i in 0..10 {
map.insert(i, ());
}

// The binary search will want to insert this at the end (index == len()),
// but that's only possible for *new* inserts. It should still be handled
// without panicking though, and in this case it's simple enough that we
// know the exact result. (But don't read this as an API guarantee!)
assert_eq!(map.first(), Some((&10, &())));
map.insert_sorted(10, ());
assert_eq!(map.last(), Some((&10, &())));
assert!(map.keys().copied().eq(0..=10));

// Other out-of-order entries can also "insert" to a binary-searched
// position, moving in either direction.
map.move_index(5, 0);
map.move_index(6, 10);
assert_eq!(map.first(), Some((&5, &())));
assert_eq!(map.last(), Some((&6, &())));
map.insert_sorted(5, ()); // moves back up
map.insert_sorted(6, ()); // moves back down
assert!(map.keys().copied().eq(0..=10));
}

#[test]
fn grow() {
let insert = [0, 4, 2, 12, 8, 7, 11];
Expand Down
2 changes: 1 addition & 1 deletion src/set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ where
///
/// This is equivalent to finding the position with
/// [`binary_search`][Self::binary_search], and if needed calling
/// [`shift_insert`][Self::shift_insert] for a new value.
/// [`insert_before`][Self::insert_before] for a new value.
///
/// If the sorted item is found in the set, it returns the index of that
/// existing item and `false`, without any change. Otherwise, it inserts the
Expand Down

0 comments on commit 8ca01b0

Please sign in to comment.