From b7057e1785979c98ea14ca86af459eecee5c335b Mon Sep 17 00:00:00 2001 From: Simon Giraudot Date: Wed, 20 Jan 2021 10:14:35 +0100 Subject: [PATCH] Make Node a const range and fix encapsulation --- Orthtree/include/CGAL/Orthtree/Node.h | 95 +++++++++++-------- Orthtree/test/Orthtree/test_octree_refine.cpp | 20 ++-- 2 files changed, 65 insertions(+), 50 deletions(-) diff --git a/Orthtree/include/CGAL/Orthtree/Node.h b/Orthtree/include/CGAL/Orthtree/Node.h index b156d4c80e6..6ca5bfed00c 100644 --- a/Orthtree/include/CGAL/Orthtree/Node.h +++ b/Orthtree/include/CGAL/Orthtree/Node.h @@ -25,15 +25,38 @@ namespace CGAL { +/// \cond SKIP_IN_MANUAL +namespace Orthtrees +{ + +// Non-documented, or testing purpose only +struct Node_access +{ + template + static Node create_node (Node parent, LC local_coordinates) + { + return Node(parent, local_coordinates); + } + + template + static typename Node::Point_range& points(Node node) { return node.points(); } + + template + static void split(Node node) { return node.split(); } +}; + +} // namespace Orthtrees +/// \endcond + /*! \brief represents a single node of the tree. Alternatively referred to as a cell, orthant, or subtree. A `Node` is a lightweight object and thus generally passed by - copy. It is also a `Range` whose value type is `Traits::Point_d`. + copy. It is also a `ConstRange` whose value type is `Traits::Point_d`. - \cgalModels Range + \cgalModels ConstRange */ template class Orthtree::Node @@ -81,16 +104,8 @@ public: typedef std::array Global_coordinates; - typedef typename PointRange::iterator iterator; ///< iterator type. typedef typename PointRange::const_iterator const_iterator; ///< constant iterator type. - /// \cond SKIP_IN_MANUAL - /*! - \brief point range type. - */ - typedef boost::iterator_range Point_range; - /// \endcond - /*! \brief easy access to adjacency directions. */ @@ -100,6 +115,14 @@ public: private: + /// \cond SKIP_IN_MANUAL + /*! + \brief point range type. + */ + typedef typename PointRange::iterator iterator; ///< constant iterator type. + typedef boost::iterator_range Point_range; + /// \endcond + // make Node trivially copiabled struct Data { @@ -115,16 +138,26 @@ private: std::shared_ptr m_data; -public: /// \cond SKIP_IN_MANUAL + // Only the Orthtree class has access to the non-default + // constructor, mutators, etc. + friend Enclosing; + + // Hidden class to access methods for testing purposes + friend Orthtrees::Node_access; + + /*! + * \brief access to the content held by this node + * \return a reference to the collection of point indices + */ + Point_range &points() { return m_data->points; } + const Point_range &points() const { return m_data->points; } + /// \name Construction /// @{ - // Default creates null node - Node() { } - /*! \brief creates a new node, optionally as the child of a parent @@ -195,6 +228,14 @@ public: /// \endcond + +public: + + /// \cond SKIP_IN_MANUAL + // Default creates null node + Node() { } + /// \endcond + /// \name Type & Location /// @{ @@ -450,20 +491,6 @@ public: /// \name Point Range /// @{ - /// \cond SKIP_IN_MANUAL - /*! - * \brief access to the content held by this node - * \return a reference to the collection of point indices - */ - Point_range &points() { return m_data->points; } - /*! - * \brief read-only access to the content held by this node - * \return a read-only reference to the collection of point indices - */ - const Point_range &points() const { return m_data->points; } - - /// \endcond - /*! \brief returns the number of points of this node. */ @@ -478,18 +505,6 @@ public: return std::distance(m_data->points.begin(), m_data->points.end()); } - /*! - \brief returns the iterator at the start of the collection of - points held by this node. - */ - iterator begin() { return m_data->points.begin(); } - - /*! - \brief returns the iterator at the end of the collection of - points held by this node. - */ - iterator end() { return m_data->points.end(); } - /*! \brief returns the iterator at the start of the collection of points held by this node. diff --git a/Orthtree/test/Orthtree/test_octree_refine.cpp b/Orthtree/test/Orthtree/test_octree_refine.cpp index 14ec3e70a94..b1326519135 100644 --- a/Orthtree/test/Orthtree/test_octree_refine.cpp +++ b/Orthtree/test/Orthtree/test_octree_refine.cpp @@ -11,8 +11,7 @@ typedef CGAL::Simple_cartesian Kernel; typedef Kernel::Point_3 Point; typedef CGAL::Point_set_3 Point_set; -typedef CGAL::Octree -Octree; +typedef CGAL::Octree Octree; typedef Octree::Node Node; void test_1_point() { @@ -26,8 +25,9 @@ void test_1_point() { octree.refine(10, 1); // Check that the topology matches - Node single_node(Node(), 0); - single_node.points() = octree.root().points(); + Node single_node = CGAL::Orthtrees::Node_access::create_node(Node(), 0); + CGAL::Orthtrees::Node_access::points(single_node) + = CGAL::Orthtrees::Node_access::points(octree.root()); assert(Node::is_topology_equal(single_node, octree.root())); assert(0 == octree.depth()); @@ -45,8 +45,8 @@ void test_2_points() { octree.refine(10, 1); // The octree should have been split once - Node other(Node(), 0); - other.split(); + Node other = CGAL::Orthtrees::Node_access::create_node(Node(), 0); + CGAL::Orthtrees::Node_access::split(other); assert(Node::is_topology_equal(other, octree.root())); assert(1 == octree.depth()); @@ -65,10 +65,10 @@ void test_4_points() { octree.refine(10, 1); // The octree should have been split once on the first level, and twice on the second - Node other(Node(), 0); - other.split(); - other[3].split(); - other[7].split(); + Node other = CGAL::Orthtrees::Node_access::create_node(Node(), 0); + CGAL::Orthtrees::Node_access::split(other); + CGAL::Orthtrees::Node_access::split(other[3]); + CGAL::Orthtrees::Node_access::split(other[7]); assert(Node::is_topology_equal(other, octree.root())); assert(2 == octree.depth()); }