From d073681630b317d251deba0041d4a0551fcb72d2 Mon Sep 17 00:00:00 2001 From: Mathew Sutcliffe Date: Tue, 28 Jan 2014 01:50:30 +0000 Subject: [PATCH 1/7] minor doxygen fixes --- src/blackmisc/containerbase.h | 4 +--- src/blackmisc/sequence.h | 5 +++++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/blackmisc/containerbase.h b/src/blackmisc/containerbase.h index ed9b7b1b8..7586938eb 100644 --- a/src/blackmisc/containerbase.h +++ b/src/blackmisc/containerbase.h @@ -86,7 +86,7 @@ namespace BlackMisc } /*! - * \brief Return true if there is an element for which a given predicate returns true + * \brief Return true if there is an element for which a given predicate returns true. */ template bool contains(Predicate p) const @@ -96,8 +96,6 @@ namespace BlackMisc /*! * \brief Return true if there is an element equal to given object - * \param object is this object in container? - * \return */ bool contains(const T &object) const { diff --git a/src/blackmisc/sequence.h b/src/blackmisc/sequence.h index f706e3c83..500fb4343 100644 --- a/src/blackmisc/sequence.h +++ b/src/blackmisc/sequence.h @@ -249,6 +249,7 @@ namespace BlackMisc /*! * \brief Remove elements for which a given predicate returns true. + * \pre The sequence must be initialized. */ template void removeIf(Predicate p) @@ -269,6 +270,7 @@ namespace BlackMisc /*! * \brief Remove the given object, if it is contained. + * \pre The sequence must be initialized. */ void remove(const T &object) { @@ -305,6 +307,7 @@ namespace BlackMisc /*! * \brief Replace elements for which a given predicate returns true. If there is no match, push the new element on the end. + * \pre The sequence must be initialized. */ template void replaceOrAdd(Predicate p, const T &replacement) @@ -315,6 +318,7 @@ namespace BlackMisc /*! * \brief Replace elements matching the given element. If there is no match, push the new element on the end. + * \pre The sequence must be initialized. */ void replaceOrAdd(const T &original, const T &replacement) { @@ -326,6 +330,7 @@ namespace BlackMisc * \brief Replace elements matching a particular key/value pair. If there is no match, push the new element on the end. * \param key1 A pointer to a member function of T. * \param value1 Will be compared to the return value of key1. + * \pre The sequence must be initialized. */ template void replaceOrAdd(K1 key1, V1 value1, const T &replacement) From 1e153b45f0180f90ddb21ff0902bba729d572213 Mon Sep 17 00:00:00 2001 From: Mathew Sutcliffe Date: Tue, 28 Jan 2014 18:25:14 +0000 Subject: [PATCH 2/7] refs #108, added find methods in CSequence and CCollection; CSequence uses std::find, and CCollection uses the more efficient find method of the implementation type, which requires that the implementation type be an associative container like QSet or std::set. Modified CContainerBase::contains to use these new find methods. --- src/blackmisc/collection.h | 32 ++++++++++++++++++++---------- src/blackmisc/containerbase.h | 4 ++-- src/blackmisc/iterator.h | 2 +- src/blackmisc/sequence.h | 5 +++++ tests/blackmisc/testcontainers.cpp | 8 +++++--- 5 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/blackmisc/collection.h b/src/blackmisc/collection.h index 2fe56dc2e..59253850a 100644 --- a/src/blackmisc/collection.h +++ b/src/blackmisc/collection.h @@ -172,6 +172,20 @@ namespace BlackMisc */ iterator erase(iterator it1, iterator it2) { Q_ASSERT(pimpl()); return pimpl()->erase(it1, it2); } + /*! + * \brief Efficient find method using the find of the implementation container. Typically O(log n). + * \return An iterator to the position of the found element, or the end iterator if not found. + * \pre The sequence must be initialized. + */ + iterator find(const T &value) { Q_ASSERT(pimpl()); return pimpl()->find(value); } + + /*! + * \brief Efficient find method using the find of the implementation container. Typically O(log n). + * \return An iterator to the position of the found element, or the end iterator if not found. + * \pre The sequence must be initialized. + */ + const_iterator find(const T &value) const { Q_ASSERT(pimpl()); return pimpl()->find(value); } + /*! * \brief Test for equality. * \todo Improve inefficient implementation. @@ -203,16 +217,9 @@ namespace BlackMisc virtual iterator insert(const T &value) = 0; virtual iterator erase(iterator pos) = 0; virtual iterator erase(iterator it1, iterator it2) = 0; + virtual iterator find(const T &value) = 0; + virtual const_iterator find(const T &value) const = 0; virtual bool operator ==(const PimplBase &other) const = 0; - protected: - // using SFINAE to choose whether to implement insert() in terms of either push_back() or insert(), depending on which is available - // https://groups.google.com/forum/#!original/comp.lang.c++.moderated/T3x6lvmvvkQ/mfY5VTDJ--UJ - class yes { char x; }; class no { yes x[2]; }; template struct typecheck {}; - struct base { void push_back(); }; template struct derived : public C, public base {}; - static yes hasPushHelper(...); template static no hasPushHelper(D *, typecheck * = 0); - template struct hasPush : public std::integral_constant*)0)) == sizeof(yes)> {}; - template static iterator insertImpl(typename std::enable_if< hasPush::value, C>::type &c, const T &value) { c.push_back(value); return iterator::fromImpl(c.end() - 1); } - template static iterator insertImpl(typename std::enable_if < !hasPush::value, C >::type &c, const T &value) { return iterator::fromImpl(c.insert(value)); } }; template class Pimpl : public PimplBase @@ -231,13 +238,18 @@ namespace BlackMisc size_type size() const { return m_impl.size(); } bool empty() const { return m_impl.empty(); } void clear() { m_impl.clear(); } - iterator insert(const T &value) { return PimplBase::insertImpl(m_impl, value); } + iterator insert(const T &value) { return iterator::fromImpl(insertHelper(m_impl.insert(value))); } iterator erase(iterator pos) { return iterator::fromImpl(m_impl.erase(*static_cast(pos.getImpl()))); } //iterator erase(iterator it1, iterator it2) { return iterator::fromImpl(m_impl.erase(*static_cast(it1.getImpl()), *static_cast(it2.getImpl()))); } iterator erase(iterator it1, iterator it2) { while (it1 != it2) { it1 = iterator::fromImpl(m_impl.erase(*static_cast(it1.getImpl()))); } return it1; } + iterator find(const T &value) { return iterator::fromImpl(m_impl.find(value)); } + const_iterator find(const T &value) const { return const_iterator::fromImpl(m_impl.find(value)); } bool operator ==(const PimplBase &other) const { Pimpl copy = C(); for (auto i = other.cbegin(); i != other.cend(); ++i) copy.insert(*i); return m_impl == copy.m_impl; } private: C m_impl; + // insertHelper: QSet::insert returns an iterator, but std::set::insert returns a std::pair + template static I insertHelper(I i) { return i; } + template static I insertHelper(std::pair p) { return p.first; } }; typedef QScopedPointer PimplPtr; diff --git a/src/blackmisc/containerbase.h b/src/blackmisc/containerbase.h index 7586938eb..6873b33ba 100644 --- a/src/blackmisc/containerbase.h +++ b/src/blackmisc/containerbase.h @@ -95,11 +95,11 @@ namespace BlackMisc } /*! - * \brief Return true if there is an element equal to given object + * \brief Return true if there is an element equal to given object. Uses the most efficient implementation available. */ bool contains(const T &object) const { - return std::find(derived().begin(), derived().end(), object) != derived().end(); + return derived().find(object) != derived().end(); } /*! diff --git a/src/blackmisc/iterator.h b/src/blackmisc/iterator.h index 56b2c9d2d..06a96e27b 100644 --- a/src/blackmisc/iterator.h +++ b/src/blackmisc/iterator.h @@ -152,7 +152,7 @@ namespace BlackMisc virtual PimplBase *clone() const { return new Pimpl(*this); } virtual const_reference operator *() const { return *m_impl; } virtual void operator ++() { ++m_impl; } - virtual void operator +=(difference_type n) { m_impl += n; } + virtual void operator +=(difference_type n) { std::advance(m_impl, n); } virtual bool operator ==(const PimplBase &other) const { return m_impl == static_cast(other).m_impl; } virtual void *impl() { return &m_impl; } private: diff --git a/src/blackmisc/sequence.h b/src/blackmisc/sequence.h index 500fb4343..1b2a09e45 100644 --- a/src/blackmisc/sequence.h +++ b/src/blackmisc/sequence.h @@ -219,6 +219,11 @@ namespace BlackMisc */ iterator erase(iterator it1, iterator it2) { Q_ASSERT(pimpl()); return pimpl()->erase(it1, it2); } + /*! + * \brief Return an iterator to the first element equal to the given object, or the end iterator if not found. O(n). + */ + iterator find(const T &object) const { return std::find(begin(), end(), object); } + /*! * \brief Modify by applying a value map to each element for which a given predicate returns true. */ diff --git a/tests/blackmisc/testcontainers.cpp b/tests/blackmisc/testcontainers.cpp index fbd73612c..b4815cbc0 100644 --- a/tests/blackmisc/testcontainers.cpp +++ b/tests/blackmisc/testcontainers.cpp @@ -7,8 +7,10 @@ #include "blackmisc/collection.h" #include "blackmisc/sequence.h" #include -#include +#include +#include #include +#include using namespace BlackMisc; @@ -19,9 +21,9 @@ namespace BlackMiscTest { CCollection c1; QVERIFY2(c1.empty(), "Uninitialized collection is empty"); - auto c2 = CCollection::fromImpl(QList()); + auto c2 = CCollection::fromImpl(QSet()); QVERIFY2(c1 == c2, "Uninitialized and empty collections are equal"); - c1.changeImpl(std::vector()); + c1.changeImpl(std::set()); QVERIFY2(c1 == c2, "Two empty collections are equal"); c1.insert(1); QVERIFY2(c1 != c2, "Different collections are not equal"); From d48d8ed9516d7e6a2b4428b7c1a8d5d5868a4816 Mon Sep 17 00:00:00 2001 From: Mathew Sutcliffe Date: Tue, 28 Jan 2014 18:30:06 +0000 Subject: [PATCH 3/7] refs #106, moved removeIf from CSequence to CContainerBase and added remove method in CCollection (CSequence already had a remove method) --- src/blackmisc/collection.h | 6 ++++++ src/blackmisc/containerbase.h | 24 ++++++++++++++++++++++++ src/blackmisc/sequence.h | 21 --------------------- 3 files changed, 30 insertions(+), 21 deletions(-) diff --git a/src/blackmisc/collection.h b/src/blackmisc/collection.h index 59253850a..d5411c687 100644 --- a/src/blackmisc/collection.h +++ b/src/blackmisc/collection.h @@ -186,6 +186,12 @@ namespace BlackMisc */ const_iterator find(const T &value) const { Q_ASSERT(pimpl()); return pimpl()->find(value); } + /*! + * \brief Efficient remove using the find and erase of the implementation container. Typically O(log n). + * \pre The sequence must be initialized. + */ + void remove(const T &object) { auto it = find(object); if (it != end()) { erase(pos); } } + /*! * \brief Test for equality. * \todo Improve inefficient implementation. diff --git a/src/blackmisc/containerbase.h b/src/blackmisc/containerbase.h index 6873b33ba..8f14dc6db 100644 --- a/src/blackmisc/containerbase.h +++ b/src/blackmisc/containerbase.h @@ -113,6 +113,30 @@ namespace BlackMisc return contains(BlackMisc::Predicates::MemberEqual(key1, value1)); } + /*! + * \brief Remove elements for which a given predicate returns true. + * \pre The sequence must be initialized. + */ + template + void removeIf(Predicate p) + { + for (auto it = derived().begin(); it != derived().end(); ++it) + { + if (p(*it)) { it = derived().erase(it); } + } + } + + /*! + * \brief Remove elements matching a particular key/value pair. + * \param key1 A pointer to a member function of T. + * \param value1 Will be compared to the return value of key1. + */ + template + void removeIf(K1 key1, V1 value1) + { + removeIf(BlackMisc::Predicates::MemberEqual(key1, value1)); + } + public: // CValueObject overrides /*! * \copydoc CValueObject::toQVariant() diff --git a/src/blackmisc/sequence.h b/src/blackmisc/sequence.h index 1b2a09e45..c7c21acd6 100644 --- a/src/blackmisc/sequence.h +++ b/src/blackmisc/sequence.h @@ -252,27 +252,6 @@ namespace BlackMisc applyIf([ & ](const T &value) { return value == pattern; }, newValues); } - /*! - * \brief Remove elements for which a given predicate returns true. - * \pre The sequence must be initialized. - */ - template - void removeIf(Predicate p) - { - erase(std::remove_if(begin(), end(), p), end()); - } - - /*! - * \brief Remove elements matching a particular key/value pair. - * \param key1 A pointer to a member function of T. - * \param value1 Will be compared to the return value of key1. - */ - template - void removeIf(K1 key1, V1 value1) - { - removeIf(BlackMisc::Predicates::MemberEqual(key1, value1)); - } - /*! * \brief Remove the given object, if it is contained. * \pre The sequence must be initialized. From 7c8aa8226dd1549c60e961d20168d325358ee05b Mon Sep 17 00:00:00 2001 From: Mathew Sutcliffe Date: Tue, 28 Jan 2014 20:19:06 +0000 Subject: [PATCH 4/7] const corrections --- src/blackmisc/collection.h | 5 +++-- src/blackmisc/containerbase.h | 10 +++++----- src/blackmisc/sequence.h | 16 +++++++++++----- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/blackmisc/collection.h b/src/blackmisc/collection.h index d5411c687..4a435f575 100644 --- a/src/blackmisc/collection.h +++ b/src/blackmisc/collection.h @@ -110,12 +110,12 @@ namespace BlackMisc iterator end() { return pimpl() ? pimpl()->end() : iterator(); } /*! - * \brief Returns iterator one past the end of the collection. + * \brief Returns const iterator one past the end of the collection. */ const_iterator end() const { return pimpl() ? pimpl()->end() : const_iterator(); } /*! - * \brief Returns iterator one past the end of the collection. + * \brief Returns const iterator one past the end of the collection. */ const_iterator cend() const { return pimpl() ? pimpl()->cend() : const_iterator(); } @@ -176,6 +176,7 @@ namespace BlackMisc * \brief Efficient find method using the find of the implementation container. Typically O(log n). * \return An iterator to the position of the found element, or the end iterator if not found. * \pre The sequence must be initialized. + * \warning Take care that the returned non-const iterator is not compared with a const iterator. */ iterator find(const T &value) { Q_ASSERT(pimpl()); return pimpl()->find(value); } diff --git a/src/blackmisc/containerbase.h b/src/blackmisc/containerbase.h index 8f14dc6db..8aac09834 100644 --- a/src/blackmisc/containerbase.h +++ b/src/blackmisc/containerbase.h @@ -91,7 +91,7 @@ namespace BlackMisc template bool contains(Predicate p) const { - return std::any_of(derived().begin(), derived().end(), p); + return std::any_of(derived().cbegin(), derived().cend(), p); } /*! @@ -99,7 +99,7 @@ namespace BlackMisc */ bool contains(const T &object) const { - return derived().find(object) != derived().end(); + return derived().find(object) != derived().cend(); } /*! @@ -156,7 +156,7 @@ namespace BlackMisc { QString str; // qualifying stringify with this-> to workaround bug in GCC 4.7.2 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56402 - std::for_each(derived().begin(), derived().end(), [ & ](const T &value) { str += (str.isEmpty() ? "{" : ",") + this->stringify(value, i18n); }); + std::for_each(derived().cbegin(), derived().cend(), [ & ](const T &value) { str += (str.isEmpty() ? "{" : ",") + this->stringify(value, i18n); }); if (str.isEmpty()) { str = "{"; } return str += "}"; } @@ -183,7 +183,7 @@ namespace BlackMisc //const auto &o = static_cast(other); //if (derived().size() < o.derived().size()) { return -1; } //if (derived().size() > o.derived().size()) { return 1; } - //for (auto i1 = derived().begin(), i2 = o.derived().begin(); i1 != derived().end() && i2 != o.derived().end(); ++i1, ++i2) + //for (auto i1 = derived().cbegin(), i2 = o.derived().cbegin(); i1 != derived().cend() && i2 != o.derived().cend(); ++i1, ++i2) //{ // if (*i1 < *i2) { return -1; } // if (*i1 > *i2) { return 1; } @@ -194,7 +194,7 @@ namespace BlackMisc virtual void marshallToDbus(QDBusArgument &argument) const { argument.beginArray(qMetaTypeId()); - std::for_each(derived().begin(), derived().end(), [ & ](const T &value) { argument << value; }); + std::for_each(derived().cbegin(), derived().cend(), [ & ](const T &value) { argument << value; }); argument.endArray(); } diff --git a/src/blackmisc/sequence.h b/src/blackmisc/sequence.h index c7c21acd6..95a2e7a48 100644 --- a/src/blackmisc/sequence.h +++ b/src/blackmisc/sequence.h @@ -95,12 +95,12 @@ namespace BlackMisc iterator begin() { return pimpl() ? pimpl()->begin() : iterator(); } /*! - * \brief Returns iterator at the beginning of the sequence. + * \brief Returns const iterator at the beginning of the sequence. */ const_iterator begin() const { return pimpl() ? pimpl()->begin() : const_iterator(); } /*! - * \brief Returns iterator at the beginning of the sequence. + * \brief Returns const iterator at the beginning of the sequence. */ const_iterator cbegin() const { return pimpl() ? pimpl()->cbegin() : const_iterator(); } @@ -110,12 +110,12 @@ namespace BlackMisc iterator end() { return pimpl() ? pimpl()->end() : iterator(); } /*! - * \brief Returns iterator one past the end of the sequence. + * \brief Returns const iterator one past the end of the sequence. */ const_iterator end() const { return pimpl() ? pimpl()->end() : const_iterator(); } /*! - * \brief Returns iterator one past the end of the sequence. + * \brief Returns const iterator one past the end of the sequence. */ const_iterator cend() const { return pimpl() ? pimpl()->cend() : const_iterator(); } @@ -221,8 +221,14 @@ namespace BlackMisc /*! * \brief Return an iterator to the first element equal to the given object, or the end iterator if not found. O(n). + * \warning Take care that the returned non-const iterator is not compared with a const iterator. */ - iterator find(const T &object) const { return std::find(begin(), end(), object); } + iterator find(const T &object) { return std::find(begin(), end(), object); } + + /*! + * \brief Return an iterator to the first element equal to the given object, or the end iterator if not found. O(n). + */ + const_iterator find(const T &object) const { return std::find(cbegin(), cend(), object); } /*! * \brief Modify by applying a value map to each element for which a given predicate returns true. From c27da2e38ac57f079515e4732248a87840569bc5 Mon Sep 17 00:00:00 2001 From: Mathew Sutcliffe Date: Tue, 28 Jan 2014 20:30:50 +0000 Subject: [PATCH 5/7] refs #108, added CContainerBase::to, for converting between different container types --- src/blackmisc/containerbase.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/blackmisc/containerbase.h b/src/blackmisc/containerbase.h index 8aac09834..c35d01db9 100644 --- a/src/blackmisc/containerbase.h +++ b/src/blackmisc/containerbase.h @@ -27,6 +27,18 @@ namespace BlackMisc class CContainerBase : public CValueObject { public: + /*! + * \brief Return a new container of a different type, containing the same elements as this one. + * \tparam Other the type of the new container. + * \param other an optional initial value for the new container; will be copied. + */ + template