From f774b270244352df5949ee0f5f0d44c64218f825 Mon Sep 17 00:00:00 2001 From: "Alexander J. Pfleger" <70842573+AJPfleger@users.noreply.github.com> Date: Tue, 10 Sep 2024 22:57:17 +0200 Subject: [PATCH] refactor: modernise GeometryHierarchyMap (#3594) - remove template `iterator_t` - remove iterators - range based loop - pass heavy objects by reference - readability --- .../Acts/Geometry/GeometryHierarchyMap.hpp | 70 ++++++++++--------- 1 file changed, 38 insertions(+), 32 deletions(-) diff --git a/Core/include/Acts/Geometry/GeometryHierarchyMap.hpp b/Core/include/Acts/Geometry/GeometryHierarchyMap.hpp index 1106761d372..b2b10b9e52f 100644 --- a/Core/include/Acts/Geometry/GeometryHierarchyMap.hpp +++ b/Core/include/Acts/Geometry/GeometryHierarchyMap.hpp @@ -64,13 +64,13 @@ class GeometryHierarchyMap { /// Combined geometry identifier and value element. Only used for input. using InputElement = typename std::pair; using Iterator = typename std::vector::const_iterator; - using Size = typename std::vector::size_type; using Value = value_t; /// Construct the container from the given elements. /// /// @param elements input elements (must be unique with respect to identifier) GeometryHierarchyMap(std::vector elements); + /// Construct the container from an initializer list. /// /// @param elements input initializer list @@ -86,21 +86,25 @@ class GeometryHierarchyMap { /// Return an iterator pointing to the beginning of the stored values. Iterator begin() const { return m_values.begin(); } + /// Return an iterator pointing to the end of the stored values. Iterator end() const { return m_values.end(); } + /// Check if any elements are stored. bool empty() const { return m_values.empty(); } + /// Return the number of stored elements. - Size size() const { return m_values.size(); } + std::size_t size() const { return m_values.size(); } /// Access the geometry identifier for the i-th element with bounds check. /// /// @throws std::out_of_range for invalid indices - GeometryIdentifier idAt(Size index) const { return m_ids.at(index); } + GeometryIdentifier idAt(std::size_t index) const { return m_ids.at(index); } + /// Access the value of the i-th element in the container with bounds check. /// /// @throws std::out_of_range for invalid indices - const Value& valueAt(Size index) const { return m_values.at(index); } + const Value& valueAt(std::size_t index) const { return m_values.at(index); } /// Find the most specific value for a given geometry identifier. /// @@ -111,7 +115,7 @@ class GeometryHierarchyMap { /// @param id geometry identifier for which information is requested /// @retval iterator to an existing value /// @retval `.end()` iterator if no matching element exists - Iterator find(GeometryIdentifier id) const; + Iterator find(const GeometryIdentifier& id) const; private: // NOTE this class assumes that it knows the ordering of the levels within @@ -171,25 +175,26 @@ class GeometryHierarchyMap { // no valid levels; all bits are zero. return Identifier{0u}; } + /// Construct a mask where only the highest level is set. static constexpr Identifier makeHighestLevelMask() { return makeLeadingLevelsMask(GeometryIdentifier(0u).setVolume(1u)); } + /// Compare the two identifiers only within the masked bits. static constexpr bool equalWithinMask(Identifier lhs, Identifier rhs, Identifier mask) { return (lhs & mask) == (rhs & mask); } + /// Ensure identifier ordering and uniqueness. - template - static void sortAndCheckDuplicates(iterator_t beg, iterator_t end); + static void sortAndCheckDuplicates(std::vector& elements); /// Fill the container from the input elements. /// /// This assumes that the elements are ordered and unique with respect to /// their identifiers. - template - void fill(iterator_t beg, iterator_t end); + void fill(const std::vector& elements); }; // implementations @@ -197,8 +202,8 @@ class GeometryHierarchyMap { template inline GeometryHierarchyMap::GeometryHierarchyMap( std::vector elements) { - sortAndCheckDuplicates(elements.begin(), elements.end()); - fill(elements.begin(), elements.end()); + sortAndCheckDuplicates(elements); + fill(elements); } template @@ -208,43 +213,44 @@ inline GeometryHierarchyMap::GeometryHierarchyMap( std::vector(elements.begin(), elements.end())) {} template -template inline void GeometryHierarchyMap::sortAndCheckDuplicates( - iterator_t beg, iterator_t end) { + std::vector& elements) { // ensure elements are sorted by identifier - std::sort(beg, end, [=](const auto& lhs, const auto& rhs) { + std::ranges::sort(elements, [=](const auto& lhs, const auto& rhs) { return lhs.first < rhs.first; }); - // check that all elements have unique identifier - auto dup = std::adjacent_find(beg, end, [](const auto& lhs, const auto& rhs) { - return lhs.first == rhs.first; - }); - if (dup != end) { + + // Check that all elements have unique identifier + auto dup = std::ranges::adjacent_find( + elements, + [](const auto& lhs, const auto& rhs) { return lhs.first == rhs.first; }); + + if (dup != elements.end()) { throw std::invalid_argument("Input elements contain duplicates"); } } template -template -inline void GeometryHierarchyMap::fill(iterator_t beg, - iterator_t end) { - const auto n = std::distance(beg, end); +inline void GeometryHierarchyMap::fill( + const std::vector& elements) { m_ids.clear(); - m_ids.reserve(n); m_masks.clear(); - m_masks.reserve(n); m_values.clear(); - m_values.reserve(n); - for (; beg != end; ++beg) { - m_ids.push_back(beg->first.value()); - m_masks.push_back(makeLeadingLevelsMask(beg->first.value())); - m_values.push_back(std::move(beg->second)); + + m_ids.reserve(elements.size()); + m_masks.reserve(elements.size()); + m_values.reserve(elements.size()); + + for (const auto& element : elements) { + m_ids.push_back(element.first.value()); + m_masks.push_back(makeLeadingLevelsMask(element.first.value())); + m_values.push_back(std::move(element.second)); } } template -inline auto GeometryHierarchyMap::find(GeometryIdentifier id) const - -> Iterator { +inline auto GeometryHierarchyMap::find( + const GeometryIdentifier& id) const -> Iterator { assert((m_ids.size() == m_values.size()) && "Inconsistent container state: #ids != # values"); assert((m_masks.size() == m_values.size()) &&