Merge pull request iwburns#27 from clbarnes/master

Remove unreachables from pub methods in Tree

Author: Ian Burns

src/iter.rs

@@ -81,7 +81,7 @@ impl<'a, T> Iterator for PreOrder<'a, T> {
                 .push(NextSiblings::new(first_child_id, self.tree));
             Some(node)
         } else {
-            while self.children.len() > 0 {
+            while !self.children.is_empty() {
                 if let Some(node_ref) = self.children.last_mut().and_then(Iterator::next) {
                     if let Some(first_child) = node_ref.first_child() {
                         self.children
@@ -197,26 +197,23 @@ impl<'a, T> Iterator for LevelOrder<'a, T> {
                         let first_child_id = next.first_child().map(|child| child.node_id());
                         self.levels
                             .push((next.node_id(), NextSiblings::new(first_child_id, self.tree)));
-                    } else {
-                        if level == 1 {
-                            if on_level < next_level {
-                                on_level += 1;
-                                let node = self
-                                    .tree
-                                    .get(node_id)
-                                    .expect("getting node of existing node ref id");
-                                let first_child_id =
-                                    node.first_child().map(|child| child.node_id());
-                                self.levels.push((
-                                    node.node_id(),
-                                    NextSiblings::new(first_child_id, self.tree),
-                                ));
-                            } else {
-                                break;
-                            }
+                    } else if level == 1 {
+                        if on_level < next_level {
+                            on_level += 1;
+                            let node = self
+                                .tree
+                                .get(node_id)
+                                .expect("getting node of existing node ref id");
+                            let first_child_id = node.first_child().map(|child| child.node_id());
+                            self.levels.push((
+                                node.node_id(),
+                                NextSiblings::new(first_child_id, self.tree),
+                            ));
                         } else {
-                            level -= 1;
+                            break;
                         }
+                    } else {
+                        level -= 1;
                     }
                 }
             }

src/lib.rs

@@ -33,26 +33,23 @@
 //! ```
 //! use slab_tree::*;
 //!
-//! fn main() {
-//!
-//!     //      "hello"
-//!     //        / \
-//!     // "world"   "trees"
-//!     //              |
-//!     //            "are"
-//!     //              |
-//!     //            "cool"
-//!
-//!     let mut tree = TreeBuilder::new().with_root("hello").build();
-//!     let root_id = tree.root_id().expect("root doesn't exist?");
-//!     let mut hello = tree.get_mut(root_id).unwrap();
-//!
-//!     hello.append("world");
-//!     hello
-//!         .append("trees")
-//!         .append("are")
-//!         .append("cool");
-//! }
+//! //      "hello"
+//! //        / \
+//! // "world"   "trees"
+//! //              |
+//! //            "are"
+//! //              |
+//! //            "cool"
+//!
+//! let mut tree = TreeBuilder::new().with_root("hello").build();
+//! let root_id = tree.root_id().expect("root doesn't exist?");
+//! let mut hello = tree.get_mut(root_id).unwrap();
+//!
+//! hello.append("world");
+//! hello
+//!     .append("trees")
+//!     .append("are")
+//!     .append("cool");
 //! ```
 //!
 //! ## `NodeId`s

src/tree.rs

@@ -337,7 +337,7 @@ impl<T> Tree<T> {
             }
             self.core_tree.remove(node_id)
         } else {
-            unreachable!();
+            None
         }
     }
 
@@ -486,7 +486,7 @@ impl<T> Tree<T> {
                 })
                 .unwrap_or((false, false))
         } else {
-            unreachable!()
+            (false, false)
         }
     }
 }
@@ -744,6 +744,15 @@ mod tree_tests {
         assert!(five.is_none());
     }
 
+    /// Test that there is no panic if caller tries to remove a removed node
+    #[test]
+    fn address_dropped() {
+        let mut tree = TreeBuilder::new().with_root(1).build();
+        let two_id = tree.root_mut().expect("root doesn't exist").node_id();
+        tree.remove(two_id, DropChildren);
+        tree.remove(two_id, DropChildren);
+    }
+
     #[test]
     fn remove_orphan() {
         let mut tree = TreeBuilder::new().with_root(1).build();