Merge pull request #176 from cuviper/move_index

Add move_index to change the position of an entry

Author: cuviper

RELEASES.md

@@ -19,6 +19,10 @@
   - The new `IndexMap::shrink_to` and `IndexSet::shrink_to` methods shrink
     the capacity with a lower bound.
 
+  - The new `IndexMap::move_index` and `IndexSet::move_index` methods change
+    the position of an item from one index to another, shifting the items
+    between to accommodate the move.
+
   - The new `map::Slice<K, V>` and `set::Slice<T>` offer a linear view of maps
     and sets, behaving a lot like normal `[(K, V)]` and `[T]` slices. Notably,
     comparison traits like `Eq` only consider items in order, rather than hash

src/map.rs

@@ -876,6 +876,19 @@ impl<K, V, S> IndexMap<K, V, S> {
         self.core.shift_remove_index(index)
     }
 
+    /// Moves the position of a key-value pair from one index to another
+    /// by shifting all other pairs in-between.
+    ///
+    /// * If `from < to`, the other pairs will shift down while the targeted pair moves up.
+    /// * If `from > to`, the other pairs will shift up while the targeted pair moves down.
+    ///
+    /// ***Panics*** if `from` or `to` are out of bounds.
+    ///
+    /// Computes in **O(n)** time (average).
+    pub fn move_index(&mut self, from: usize, to: usize) {
+        self.core.move_index(from, to)
+    }
+
     /// Swaps the position of two key-value pairs in the map.
     ///
     /// ***Panics*** if `a` or `b` are out of bounds.

src/map/core.rs

@@ -283,29 +283,77 @@ impl<K, V> IndexMapCore<K, V> {
     ///
     /// The index should already be removed from `self.indices`.
     fn shift_remove_finish(&mut self, index: usize) -> (K, V) {
-        // use Vec::remove, but then we need to update the indices that point
-        // to all of the other entries that have to move
+        // Correct indices that point to the entries that followed the removed entry.
+        self.decrement_indices(index + 1, self.entries.len());
+
+        // Use Vec::remove to actually remove the entry.
         let entry = self.entries.remove(index);
+        (entry.key, entry.value)
+    }
 
-        // correct indices that point to the entries that followed the removed entry.
-        // use a heuristic between a full sweep vs. a `find()` for every shifted item.
-        let raw_capacity = self.indices.buckets();
-        let shifted_entries = &self.entries[index..];
-        if shifted_entries.len() > raw_capacity / 2 {
-            // shift all indices greater than `index`
+    /// Decrement all indices in the range `start..end`.
+    ///
+    /// The index `start - 1` should not exist in `self.indices`.
+    /// All entries should still be in their original positions.
+    fn decrement_indices(&mut self, start: usize, end: usize) {
+        // Use a heuristic between a full sweep vs. a `find()` for every shifted item.
+        let shifted_entries = &self.entries[start..end];
+        if shifted_entries.len() > self.indices.buckets() / 2 {
+            // Shift all indices in range.
             for i in self.indices_mut() {
-                if *i > index {
+                if start <= *i && *i < end {
                     *i -= 1;
                 }
             }
         } else {
-            // find each following entry to shift its index
-            for (i, entry) in (index + 1..).zip(shifted_entries) {
+            // Find each entry in range to shift its index.
+            for (i, entry) in (start..end).zip(shifted_entries) {
                 update_index(&mut self.indices, entry.hash, i, i - 1);
             }
         }
+    }
 
-        (entry.key, entry.value)
+    /// Increment all indices in the range `start..end`.
+    ///
+    /// The index `end` should not exist in `self.indices`.
+    /// All entries should still be in their original positions.
+    fn increment_indices(&mut self, start: usize, end: usize) {
+        // Use a heuristic between a full sweep vs. a `find()` for every shifted item.
+        let shifted_entries = &self.entries[start..end];
+        if shifted_entries.len() > self.indices.buckets() / 2 {
+            // Shift all indices in range.
+            for i in self.indices_mut() {
+                if start <= *i && *i < end {
+                    *i += 1;
+                }
+            }
+        } else {
+            // Find each entry in range to shift its index, updated in reverse so
+            // we never have duplicated indices that might have a hash collision.
+            for (i, entry) in (start..end).zip(shifted_entries).rev() {
+                update_index(&mut self.indices, entry.hash, i, i + 1);
+            }
+        }
+    }
+
+    pub(super) fn move_index(&mut self, from: usize, to: usize) {
+        let from_hash = self.entries[from].hash;
+        if from != to {
+            // Use a sentinal index so other indices don't collide.
+            update_index(&mut self.indices, from_hash, from, usize::MAX);
+
+            // Update all other indices and rotate the entry positions.
+            if from < to {
+                self.decrement_indices(from + 1, to + 1);
+                self.entries[from..=to].rotate_left(1);
+            } else if to < from {
+                self.increment_indices(to, from);
+                self.entries[to..=from].rotate_right(1);
+            }
+
+            // Change the sentinal index to its final position.
+            update_index(&mut self.indices, from_hash, usize::MAX, to);
+        }
     }
 
     /// Remove an entry by swapping it with the last

src/set.rs

@@ -731,6 +731,19 @@ impl<T, S> IndexSet<T, S> {
         self.map.shift_remove_index(index).map(|(x, ())| x)
     }
 
+    /// Moves the position of a value from one index to another
+    /// by shifting all other values in-between.
+    ///
+    /// * If `from < to`, the other values will shift down while the targeted value moves up.
+    /// * If `from > to`, the other values will shift up while the targeted value moves down.
+    ///
+    /// ***Panics*** if `from` or `to` are out of bounds.
+    ///
+    /// Computes in **O(n)** time (average).
+    pub fn move_index(&mut self, from: usize, to: usize) {
+        self.map.move_index(from, to)
+    }
+
     /// Swaps the position of two values in the set.
     ///
     /// ***Panics*** if `a` or `b` are out of bounds.

tests/quick.rs

@@ -216,6 +216,53 @@ quickcheck_limit! {
             map[&key] == value && map[i] == value
         })
     }
+
+    // Use `u8` test indices so quickcheck is less likely to go out of bounds.
+    fn swap_indices(vec: Vec<u8>, a: u8, b: u8) -> TestResult {
+        let mut set = IndexSet::<u8>::from_iter(vec);
+        let a = usize::from(a);
+        let b = usize::from(b);
+
+        if a >= set.len() || b >= set.len() {
+            return TestResult::discard();
+        }
+
+        let mut vec = Vec::from_iter(set.iter().cloned());
+        vec.swap(a, b);
+
+        set.swap_indices(a, b);
+
+        // Check both iteration order and hash lookups
+        assert!(set.iter().eq(vec.iter()));
+        assert!(vec.iter().enumerate().all(|(i, x)| {
+            set.get_index_of(x) == Some(i)
+        }));
+        TestResult::passed()
+    }
+
+    // Use `u8` test indices so quickcheck is less likely to go out of bounds.
+    fn move_index(vec: Vec<u8>, from: u8, to: u8) -> TestResult {
+        let mut set = IndexSet::<u8>::from_iter(vec);
+        let from = usize::from(from);
+        let to = usize::from(to);
+
+        if from >= set.len() || to >= set.len() {
+            return TestResult::discard();
+        }
+
+        let mut vec = Vec::from_iter(set.iter().cloned());
+        let x = vec.remove(from);
+        vec.insert(to, x);
+
+        set.move_index(from, to);
+
+        // Check both iteration order and hash lookups
+        assert!(set.iter().eq(vec.iter()));
+        assert!(vec.iter().enumerate().all(|(i, x)| {
+            set.get_index_of(x) == Some(i)
+        }));
+        TestResult::passed()
+    }
 }
 
 use crate::Op::*;