FEAT: Change .insert() and .remove() to match Vec

- Add try_insert / try_remove to be the old fallible variants that
  return errors
- Insert that pushes out if full does no longer exist -- full vec is an
  error

Cargo.toml

@@ -27,6 +27,9 @@ default-features = false
 [dev-dependencies.serde_test]
 version = "1.0"
 
+[dev-dependencies]
+matches = { version = "0.1" }
+
 [features]
 default = ["std"]
 std = ["odds/std", "nodrop/std"]

src/lib.rs

@@ -209,33 +209,54 @@ impl<A: Array> ArrayVec<A> {
 
     /// Insert `element` in position `index`.
     ///
-    /// Shift up all elements after `index`. If any is pushed out, it is returned.
+    /// Shift up all elements after `index`.
     ///
-    /// Return `None` if no element is shifted out.
+    /// It is an error if the index is greater than the length or if the
+    /// arrayvec is full.
     ///
-    /// `index` must be <= `self.len()` and < `self.capacity()`. Note that any
+    /// ***Panics*** on errors. See `try_result` for fallible version.
-    /// out of bounds index insert results in the element being "shifted out"
-    /// and returned directly.
     ///
     /// ```
     /// use arrayvec::ArrayVec;
     ///
     /// let mut array = ArrayVec::<[_; 2]>::new();
     ///
-    /// assert_eq!(array.insert(0, "x"), None);
+    /// array.insert(0, "x");
-    /// assert_eq!(array.insert(0, "y"), None);
+    /// array.insert(0, "y");
-    /// assert_eq!(array.insert(0, "z"), Some("x"));
+    /// assert_eq!(&array[..], &["y", "x"]);
-    /// assert_eq!(array.insert(1, "w"), Some("y"));
-    /// assert_eq!(&array[..], &["z", "w"]);
     ///
     /// ```
-    pub fn insert(&mut self, index: usize, element: A::Item) -> Option<A::Item> {
+    pub fn insert(&mut self, index: usize, element: A::Item) {
-        if index > self.len() || index == self.capacity() {
+        self.try_insert(index, element).unwrap()
-            return Some(element);
     }
-        let mut ret = None;
+
+    /// Insert `element` in position `index`.
+    ///
+    /// Shift up all elements after `index`.
+    ///
+    /// Returns an error if
+    ///
+    /// - The vector is at full capacity
+    /// - The index is out of bounds
+    ///
+    /// ```
+    /// use arrayvec::ArrayVec;
+    ///
+    /// let mut array = ArrayVec::<[_; 2]>::new();
+    ///
+    /// assert!(array.try_insert(0, "x").is_ok());
+    /// assert!(array.try_insert(3, "w").is_err());
+    /// assert!(array.try_insert(0, "y").is_ok());
+    /// assert!(array.try_insert(0, "z").is_err());
+    /// assert_eq!(&array[..], &["y", "x"]);
+    ///
+    /// ```
+    pub fn try_insert(&mut self, index: usize, element: A::Item) -> Result<(), InsertError<A::Item>> {
         if self.len() == self.capacity() {
-            ret = self.pop();
+            return Err(InsertError::Full(element));
+        }
+        if index > self.len() {
+            return Err(InsertError::OutOfBounds);
         }
         let len = self.len();
 
@@ -253,7 +274,7 @@ impl<A: Array> ArrayVec<A> {
             }
             self.set_len(len + 1);
         }
-        ret
+        Ok(())
     }
 
     /// Remove the last element in the vector.
@@ -308,23 +329,46 @@ impl<A: Array> ArrayVec<A> {
 
     /// Remove the element at `index` and shift down the following elements.
     ///
-    /// Return `Some(` *element* `)` if the index is in bounds, else `None`.
+    /// ***Panics*** if the `index` is greater or equal to the length of the
+    /// vector.
     ///
     /// ```
     /// use arrayvec::ArrayVec;
     ///
     /// let mut array = ArrayVec::from([1, 2, 3]);
     ///
-    /// assert_eq!(array.remove(0), Some(1));
+    /// let removed_elt = array.remove(0);
+    /// assert_eq!(removed_elt, 1);
+    /// assert_eq!(&array[..], &[2, 3]);
+    /// ```
+    pub fn remove(&mut self, index: usize) -> A::Item {
+        self.try_remove(index).unwrap()
+    }
+
+    /// Remove the element at `index` and shift down the following elements.
+    ///
+    /// Returns an error if:
+    ///
+    /// - The index is greater or equal to the length of the vector
+    ///
+    /// Else return the element inside `Ok`.
+    ///
+    /// ```
+    /// use arrayvec::ArrayVec;
+    ///
+    /// let mut array = ArrayVec::from([1, 2, 3]);
+    ///
+    /// assert!(array.try_remove(0).is_ok());
     /// assert_eq!(&array[..], &[2, 3]);
     ///
-    /// assert_eq!(array.remove(10), None);
+    /// assert!(array.try_remove(2).is_err());
+    /// assert!(array.try_remove(10).is_err());
     /// ```
-    pub fn remove(&mut self, index: usize) -> Option<A::Item> {
+    pub fn try_remove(&mut self, index: usize) -> Result<A::Item, RemoveError> {
         if index >= self.len() {
-            None
+            Err(RemoveError::new())
         } else {
-            self.drain(index..index + 1).next()
+            self.drain(index..index + 1).next().ok_or_else(|| panic!())
         }
     }
 
@@ -945,3 +989,37 @@ impl<T> fmt::Debug for CapacityError<T> {
         write!(f, "{}: {}", "CapacityError", CAPERROR)
     }
 }
+
+pub enum InsertError<T> {
+    Full(T),
+    OutOfBounds,
+}
+
+impl<T> InsertError<T> {
+    fn description(&self) -> &'static str {
+        match *self {
+            InsertError::Full(_) => "ArrayVec is already at full capacity",
+            InsertError::OutOfBounds => "index is out of bounds",
+        }
+    }
+}
+
+impl<T> fmt::Debug for InsertError<T> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        match *self {
+            InsertError::Full(_) => write!(f, "InsertError::Full: ")?,
+            InsertError::OutOfBounds => write!(f, "InsertError::OutOfBounds: ")?,
+        }
+        write!(f, "{}", self.description())
+    }
+}
+
+#[derive(Debug)]
+pub struct RemoveError {
+}
+
+impl RemoveError {
+    pub fn new() -> Self {
+        RemoveError { }
+    }
+}

tests/tests.rs

@@ -1,4 +1,5 @@
 extern crate arrayvec;
+#[macro_use] extern crate matches;
 
 use arrayvec::ArrayVec;
 use arrayvec::ArrayString;
@@ -218,20 +219,23 @@ fn test_drop_panic_into_iter() {
 fn test_insert() {
     let mut v = ArrayVec::from([]);
     assert_eq!(v.push(1), Some(1));
-    assert_eq!(v.insert(0, 1), Some(1));
+    assert_matches!(v.try_insert(0, 1), Err(_));
 
     let mut v = ArrayVec::<[_; 3]>::new();
     v.insert(0, 0);
     v.insert(1, 1);
     v.insert(2, 2);
-    v.insert(3, 3);
+    let ret1 = v.try_insert(3, 3);
     assert_eq!(&v[..], &[0, 1, 2]);
-    v.insert(1, 9);
+    assert_matches!(ret1, Err(_));
-    assert_eq!(&v[..], &[0, 9, 1]);
+
+    let ret2 = v.try_insert(1, 9);
+    assert_eq!(&v[..], &[0, 1, 2]);
+    assert_matches!(ret2, Err(_));
 
     let mut v = ArrayVec::from([2]);
-    assert_eq!(v.insert(1, 1), Some(1));
+    assert_matches!(v.try_insert(1, 1), Err(_));
-    assert_eq!(v.insert(2, 1), Some(1));
+    assert_matches!(v.try_insert(2, 1), Err(_));
 }
 
 #[test]
@@ -372,28 +376,32 @@ fn test_string_push() {
 #[test]
 fn test_insert_at_length() {
     let mut v = ArrayVec::<[_; 8]>::new();
-    let result1 = v.insert(0, "a");
+    let result1 = v.try_insert(0, "a");
-    let result2 = v.insert(1, "b");
+    let result2 = v.try_insert(1, "b");
-    assert!(result1.is_none() && result2.is_none());
+    assert!(result1.is_ok() && result2.is_ok());
     assert_eq!(&v[..], &["a", "b"]);
 }
 
 #[test]
 fn test_insert_out_of_bounds() {
     let mut v = ArrayVec::<[_; 8]>::new();
-    let result = v.insert(1, "test");
+    let result = v.try_insert(1, "test");
-    assert_eq!(result, Some("test"));
+    assert_matches!(result, Err(_));
     assert_eq!(v.len(), 0);
 
-    let mut u = ArrayVec::from([1, 2, 3, 4]);
-    let ret = u.insert(3, 99);
-    assert_eq!(&u[..], &[1, 2, 3, 99]);
-    assert_eq!(ret, Some(4));
-    let ret = u.insert(4, 77);
-    assert_eq!(&u[..], &[1, 2, 3, 99]);
-    assert_eq!(ret, Some(77));
 }
 
+/*
+ * insert that pushes out the last
+    let mut u = ArrayVec::from([1, 2, 3, 4]);
+    let ret = u.try_insert(3, 99);
+    assert_eq!(&u[..], &[1, 2, 3, 99]);
+    assert_matches!(ret, Err(_));
+    let ret = u.try_insert(4, 77);
+    assert_eq!(&u[..], &[1, 2, 3, 99]);
+    assert_matches!(ret, Err(_));
+*/
+
 #[test]
 fn test_drop_in_insert() {
     use std::cell::Cell;
@@ -416,9 +424,9 @@ fn test_drop_in_insert() {
         array.push(Bump(flag));
         array.insert(0, Bump(flag));
         assert_eq!(flag.get(), 0);
-        let ret = array.insert(1, Bump(flag));
+        let ret = array.try_insert(1, Bump(flag));
         assert_eq!(flag.get(), 0);
-        assert!(ret.is_some());
+        assert_matches!(ret, Err(_));
         drop(ret);
         assert_eq!(flag.get(), 1);
     }