From 071ab82a7a10904b52f4a0253402b6978c5d1c81 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sat, 31 May 2025 01:40:36 +0200 Subject: [PATCH] clean-up: review and validate `ScopedPtrVect` This is a plausible concept, and without obvious replacement (letting aside `boost::ptr_vector`). It has a small number of usages, and provides a dedicated API to show the semantics when used as implementation of an ''Object Manager'' The original implementation used private inheritance from `std::vector`, which is not really justified here, since we neither use the ''template method'' pattern, nor want to gain access to protected internals. So this can be replaced with a private member. --- src/lib/scoped-collection.hpp | 2 +- src/lib/scoped-ptrvect.hpp | 116 +++++++++++++------------- tests/library/scoped-ptrvect-test.cpp | 2 +- wiki/thinkPad.ichthyo.mm | 106 +++++++++++++++++++---- 4 files changed, 150 insertions(+), 76 deletions(-) diff --git a/src/lib/scoped-collection.hpp b/src/lib/scoped-collection.hpp index 1fb95e857..3ccad4b0f 100644 --- a/src/lib/scoped-collection.hpp +++ b/src/lib/scoped-collection.hpp @@ -50,7 +50,7 @@ ** is sufficient to hold any of these subclass instances. This condition ** is protected by a static assertion (compilation failure). ** @warning when using subclasses, a virtual dtor is mandatory - ** @warning deliberately \em not threadsafe + ** @warning deliberately **not threadsafe** ** ** @see ScopedCollection_test ** @see scoped-ptrvect.hpp quite similar, but using individual heap pointers diff --git a/src/lib/scoped-ptrvect.hpp b/src/lib/scoped-ptrvect.hpp index a485965c7..c00456b31 100644 --- a/src/lib/scoped-ptrvect.hpp +++ b/src/lib/scoped-ptrvect.hpp @@ -1,5 +1,5 @@ /* - SCOPED-PTRVECT.hpp - simple noncopyable lifecycle managing collection of pointers + SCOPED-PTRVECT.hpp - simple noncopyable lifecycle managing collection of pointers Copyright (C) 2009, Hermann Vosseler @@ -13,20 +13,24 @@ /** @file scoped-ptrvect.hpp ** Managing lifecycle for a collection of objects. Sometimes we need to - ** build and own a number of objects, including lifecycle management. + ** build and own a number of objects, including lifecycle management. ** For example, a service provider may need to maintain a number of individual ** process handles. The solution here is deliberately kept simple, it is ** similar to using a STL container with shared_ptr(s), but behaves rather - ** like std::unique_ptr. It provides the same basic functionality as - ** boost::ptr_vector, but doesn't require us to depend on boost-serialisation. + ** like std::unique_ptr. ** ** Some details to note: ** - contained objects accessed by reference, never NULL. ** - the exposed iterator automatically dereferences - ** - TODO: detaching of objects... - ** - TODO: retro-fit with RefArray interface ** - ** @warning deliberately \em not threadsafe + ** @warning deliberately **not threadsafe** + ** @remark This library class provides the same basic functionality as + ** `boost::ptr_vector`, but doesn't require us to depend on boost-serialisation. + ** Furthermore, Boost provides several variants of containers and a much more + ** feature-rich API, also with the ability to clone / move containers; + ** yet we prefer to leave out most functionality, in favour of adding + ** a dedicated API with #manage() and #detach() to indicated semantics. + ** And we value the ability to integrate with Lumiera Forward Iterators. ** ** @see scoped-ptrvect-test.cpp ** @see scoped-holder.hpp @@ -55,23 +59,23 @@ namespace lib { /** * Simple vector based collection of pointers, * managing lifecycle of the pointed-to objects. - * Implemented as a non-copyable object, based on a - * vector of bare pointers (private inheritance) + * Implemented as a non-copyable object, based + * on a vector of bare pointers. */ template class ScopedPtrVect - : std::vector, - util::NonCopyable + : util::NonCopyable { - typedef std::vector _Vec; - typedef typename _Vec::iterator VIter; + using _Vec = std::vector; + using VIter = typename _Vec::iterator; - typedef RangeIter RIter; - typedef PtrDerefIter IterType; + using RIter = RangeIter; + using IterType = PtrDerefIter; - typedef typename IterType::ConstIterType ConstIterType; - typedef typename IterType::WrappedConstIterType RcIter; + using ConstIterType = typename IterType::ConstIterType; + using RcIter = typename IterType::WrappedConstIterType; + _Vec vec_; public: typedef size_t size_type; @@ -81,20 +85,20 @@ namespace lib { - ScopedPtrVect () - : _Vec() + ~ScopedPtrVect() + { + clear(); + } + + ScopedPtrVect() + : vec_{} { } explicit ScopedPtrVect (size_type capacity) - : _Vec() + : vec_{} { - _Vec::reserve (capacity); - } - - ~ScopedPtrVect () - { - clear(); + vec_.reserve (capacity); } @@ -107,9 +111,9 @@ namespace lib { manage (T* obj) { REQUIRE (obj); - try + try { - this->push_back (obj); + vec_.push_back (obj); return *obj; } catch(...) @@ -128,35 +132,33 @@ namespace lib { * Otherwise, NULL will be returned and the * collection of managed objects remains unaltered * @note EX_STRONG - * @todo TICKET #856 better return a Maybe instead of a pointer? */ T* detach (void* objAddress) { T* extracted = static_cast (objAddress); - VIter pos = std::find (_Vec::begin(),_Vec::end(), extracted); - if (pos != _Vec::end() && bool(*pos)) + VIter pos = std::find (vec_.begin(),vec_.end(), extracted); + if (pos != vec_.end() and *pos != nullptr) { extracted = *pos; - _Vec::erase(pos); // EX_STRONG + vec_.erase(pos); // EX_STRONG return extracted; } - return NULL; + return nullptr; } void clear() - { - VIter e = _Vec::end(); - for (VIter i = _Vec::begin(); i!=e; ++i) - if (*i) + { + for (T*& p : vec_) + if (p) try { - delete *i; - *i = 0; + delete p; + p = nullptr; } ERROR_LOG_AND_IGNORE (library, "Clean-up of ScopedPtrVect"); - _Vec::clear(); + vec_.clear(); } @@ -165,26 +167,26 @@ namespace lib { T& operator[] (size_type i) { - return *get(i); + return * get(i); } - typedef IterType iterator; - typedef ConstIterType const_iterator; + using iterator = IterType; + using const_iterator = ConstIterType; - iterator begin() { return iterator (allPtrs()); } - iterator end() { return iterator ( RIter() ); } - const_iterator begin() const { return const_iterator::build_by_cast (allPtrs()); } - const_iterator end() const { return const_iterator::nil(); } + iterator begin() { return iterator (allPtrs()); } + iterator end() { return iterator ( RIter() ); } + const_iterator begin() const { return const_iterator::build_by_cast (allPtrs()); } + const_iterator end() const { return const_iterator::nil(); } /* ====== proxied vector functions ==================== */ - size_type size () const { return _Vec::size(); } - size_type max_size () const { return _Vec::max_size(); } - size_type capacity () const { return _Vec::capacity(); } - bool empty () const { return _Vec::empty(); } + size_type size() const { return vec_.size(); } + size_type max_size() const { return vec_.max_size(); } + size_type capacity() const { return vec_.capacity(); } + bool empty() const { return vec_.empty(); } private: @@ -192,7 +194,7 @@ namespace lib { T* get (size_type i) { - T* p (_Vec::at (i)); + T* p (vec_.at (i)); if (!p) throw lumiera::error::Invalid("no valid object at this index"); else @@ -201,15 +203,15 @@ namespace lib { /** @internal access sequence of all managed pointers */ RIter - allPtrs () + allPtrs() { - return RIter (_Vec::begin(), _Vec::end()); + return RIter{vec_.begin(), vec_.end()}; } RIter - allPtrs () const + allPtrs() const { - _Vec& elements = util::unConst(*this); - return RIter (elements.begin(), elements.end()); + _Vec& elements = util::unConst(this)->vec_; + return RIter{elements.begin(), elements.end()}; } }; diff --git a/tests/library/scoped-ptrvect-test.cpp b/tests/library/scoped-ptrvect-test.cpp index 0e054c2f1..c8bfb6307 100644 --- a/tests/library/scoped-ptrvect-test.cpp +++ b/tests/library/scoped-ptrvect-test.cpp @@ -125,7 +125,7 @@ namespace test{ // Verify correct behaviour of iteration end - CHECK (! (holder.end())); + CHECK (not holder.end()); CHECK (isnil (holder.end())); VERIFY_ERROR (ITER_EXHAUST, *holder.end() ); diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index 59a7acfa9..e45fc2ba5 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -163462,11 +163462,15 @@ Since then others have made contributions, see the log for the history. - - - - - + + + + + + + + + @@ -163494,7 +163498,8 @@ Since then others have made contributions, see the log for the history. - + + @@ -163516,22 +163521,26 @@ Since then others have made contributions, see the log for the history. - + + + + - + + - + - + @@ -163549,7 +163558,7 @@ Since then others have made contributions, see the log for the history. - + @@ -163592,7 +163601,7 @@ Since then others have made contributions, see the log for the history. - + @@ -163607,6 +163616,8 @@ Since then others have made contributions, see the log for the history. + + @@ -163618,8 +163629,9 @@ Since then others have made contributions, see the log for the history. - - + + + @@ -163630,15 +163642,75 @@ Since then others have made contributions, see the log for the history. - - + - + + + + + + + + + + + + + + + + + +

+ ...was man auch daran sieht, daß es das gleiche Konezpt in Boost gibt; wäre da nicht Boost-serialisation, dann könnte man das als Ersatz nehmen +

+ + + + + + + + + + + + + + + + + + + + + + +

+ außerdem habe ich hier bereits pervasiv mit einem Typ-Präfix _Vect::  dekoriert +

+ + + + + + + + +

+ ...was darauf hindeutet, daß ich bereits Probleme mit der Eindeutigkeit von Namen hatte, bzw. die Notation ohnehin verwirrend war. +

+ + + + + +