diff options
author | vrouvrea <vrouvrea@636b058d-ea47-450e-bf9e-a15bfbe3eedb> | 2018-09-03 15:00:41 +0000 |
---|---|---|
committer | vrouvrea <vrouvrea@636b058d-ea47-450e-bf9e-a15bfbe3eedb> | 2018-09-03 15:00:41 +0000 |
commit | 527d4871c7826d5a07686c5e3f1f6edb18fbdc3d (patch) | |
tree | beba4bac3d540907b0fe1bc2a90fa5ba83cac341 /src/Simplex_tree/include | |
parent | 7f94efd0e40f9fe15d9131643c19e285fbf75d6e (diff) |
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
Diffstat (limited to 'src/Simplex_tree/include')
-rw-r--r-- | src/Simplex_tree/include/gudhi/Simplex_tree.h | 93 |
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; } |