Code review : Fix remarks for move and copy assignment

git-svn-id: svn+ssh://scm.gforge.inria.fr/svnroot/gudhi/branches/simplex_tree_fix_vincent@3868 636b058d-ea47-450e-bf9e-a15bfbe3eedb

Former-commit-id: 069e920ab3ca9504b5e1c49ab23afe1693f4e6f8

1 files changed, 52 insertions, 41 deletions

diff --git a/src/Simplex_tree/include/gudhi/Simplex_tree.h b/src/Simplex_tree/include/gudhi/Simplex_tree.h
index 69ed5e13..20b527a2 100644
--- a/src/Simplex_tree/include/gudhi/Simplex_tree.h
+++ b/src/Simplex_tree/include/gudhi/Simplex_tree.h
@@ -296,15 +296,15 @@ class Simplex_tree {
       dimension_(-1) { }
 
   /** \brief User-defined copy constructor reproduces the whole tree structure. */
-  Simplex_tree(const Simplex_tree& simplex_source)
-      : null_vertex_(simplex_source.null_vertex_),
-      root_(nullptr, null_vertex_, simplex_source.root_.members_),
+  Simplex_tree(const Simplex_tree& complex_source)
+      : null_vertex_(complex_source.null_vertex_),
+      root_(nullptr, null_vertex_, complex_source.root_.members_),
       filtration_vect_(),
-      dimension_(simplex_source.dimension_) {
+      dimension_(complex_source.dimension_) {
 #ifdef DEBUG_TRACES
     std::cout << "Simplex_tree copy constructor" << std::endl;
 #endif  // DEBUG_TRACES
-    auto root_source = simplex_source.root_;
+    auto root_source = complex_source.root_;
     rec_copy(&root_, &root_source);
   }
 
@@ -324,34 +324,32 @@ class Simplex_tree {
   }
 
   /** \brief User-defined move constructor relocates the whole tree structure.
-   *  \exception std::invalid_argument In debug mode, if the simplex_source is invalid.
+   *  \exception std::invalid_argument In debug mode, if the complex_source is invalid.
    */
-  Simplex_tree(Simplex_tree && simplex_source)
-      : null_vertex_(std::move(simplex_source.null_vertex_)),
-      root_(std::move(simplex_source.root_)),
-      filtration_vect_(std::move(simplex_source.filtration_vect_)),
-      dimension_(std::move(simplex_source.dimension_)) {
+  Simplex_tree(Simplex_tree && complex_source)
+      : null_vertex_(std::move(complex_source.null_vertex_)),
+      root_(std::move(complex_source.root_)),
+      filtration_vect_(std::move(complex_source.filtration_vect_)),
+      dimension_(std::move(complex_source.dimension_)) {
 #ifdef DEBUG_TRACES
     std::cout << "Simplex_tree move constructor" << std::endl;
 #endif  // DEBUG_TRACES
     // Need to update root members (children->oncles and children need to point on the new root pointer)
     for (auto& map_el : root_.members()) {
-      // children->oncles update after the move
-      if (map_el.second.children()->oncles() == &(simplex_source.root_))
-        // reset with the moved root_ pointer value
+      if (map_el.second.children()->oncles() == &(complex_source.root_)) {
+        // reset children->oncles with the moved root_ pointer value
         map_el.second.children()->oncles_ = &root_;
-      else
+      } else {
         // if simplex is of dimension 0, oncles_ shall be nullptr
         GUDHI_CHECK(map_el.second.children()->oncles_ == nullptr,
                     std::invalid_argument("Simplex_tree move constructor from an invalid Simplex_tree"));
-      // children update after the move
-      if (map_el.second.children() == &(simplex_source.root_))
-        // reset with the moved root_ pointer value
+        // and children points on root_ - to be moved
         map_el.second.assign_children(&root_);
+      }
     }
     // just need to set dimension_ on source to make it available again
     // (filtration_vect_ and members are already set from the move)
-    simplex_source.dimension_ = -1;
+    complex_source.dimension_ = -1;
   }
 
   /** \brief Destructor; deallocates the whole tree structure. */
@@ -364,17 +362,24 @@ class Simplex_tree {
   }
 
   /** \brief User-defined copy assignment reproduces the whole tree structure. */
-  Simplex_tree& operator= (const Simplex_tree& simplex_source)
+  Simplex_tree& operator= (const Simplex_tree& complex_source)
   {
 #ifdef DEBUG_TRACES
-    std::cout << "copy assignment" << std::endl;
+    std::cout << "Simplex_tree copy assignment" << std::endl;
 #endif  // DEBUG_TRACES
-    null_vertex_ = simplex_source.null_vertex_;
+    null_vertex_ = complex_source.null_vertex_;
     filtration_vect_.clear();
-    dimension_ = simplex_source.dimension_;
-    auto root_source = simplex_source.root_;
-    // Here a copy will be done
-    root_ = Siblings(nullptr, null_vertex_);
+    dimension_ = complex_source.dimension_;
+    auto root_source = complex_source.root_;
+
+    // We start by deleting root_ if not empty
+    for (auto sh = root_.members().begin(); sh != root_.members().end(); ++sh) {
+      if (has_children(sh)) {
+        rec_delete(sh->second.children());
+      }
+    }
+
+    // root members copy
     root_.members() = Dictionary(boost::container::ordered_unique_range, root_source.members().begin(), root_source.members().end());
     // Needs to reassign children
     for (auto& map_el : root_.members()) {
@@ -385,35 +390,41 @@ class Simplex_tree {
   }
 
   /** \brief User-defined move assignment relocates the whole tree structure.
-   *  \exception std::invalid_argument In debug mode, if the simplex_source is invalid.
+   *  \exception std::invalid_argument In debug mode, if the complex_source is invalid.
    */
-  Simplex_tree& operator=(Simplex_tree&& simplex_source)
+  Simplex_tree& operator=(Simplex_tree&& complex_source)
   {
 #ifdef DEBUG_TRACES
-    std::cout << "move assignment" << std::endl;
+    std::cout << "Simplex_tree move assignment" << std::endl;
 #endif  // DEBUG_TRACES
     // Self-assignment detection
-    if (&simplex_source != this) {
-      std::swap( null_vertex_, simplex_source.null_vertex_ );
-      std::swap( root_, simplex_source.root_ );
-      std::swap( filtration_vect_, simplex_source.filtration_vect_ );
-      std::swap( dimension_, simplex_source.dimension_ );
+    if (&complex_source != this) {
+      std::swap( null_vertex_, complex_source.null_vertex_ );
+      std::swap( root_, complex_source.root_ );
+      std::swap( filtration_vect_, complex_source.filtration_vect_ );
+      std::swap( dimension_, complex_source.dimension_ );
 
       // Need to update root members (children->oncles and children need to point on the new root pointer)
       for (auto& map_el : root_.members()) {
-        // children->oncles update after the move
-        if (map_el.second.children()->oncles() == &(simplex_source.root_))
-          // reset with the moved root_ pointer value
+        if (map_el.second.children()->oncles() == &(complex_source.root_)) {
+          // reset children->oncles with the moved root_ pointer value
           map_el.second.children()->oncles_ = &root_;
-        else
+        } else {
           // if simplex is of dimension 0, oncles_ shall be nullptr
           GUDHI_CHECK(map_el.second.children()->oncles_ == nullptr,
                       std::invalid_argument("Simplex_tree move constructor from an invalid Simplex_tree"));
-        // children update after the move
-        if (map_el.second.children() == &(simplex_source.root_))
-          // reset with the moved root_ pointer value
+          // and children points on root_ - to be moved
           map_el.second.assign_children(&root_);
+        }
+      }
+      // complex_source root_ deletion
+      for (auto sh = complex_source.root_.members().begin(); sh != complex_source.root_.members().end(); ++sh) {
+        if (has_children(sh)) {
+          rec_delete(sh->second.children());
+        }
       }
+      complex_source.root_.members().clear();
+      complex_source.dimension_ = -1;
     }
     return *this;
   }