Latest container changes

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(); }
 
@@ -172,6 +172,27 @@ 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.
+         * \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); }
+
+        /*!
+         * \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 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(it); } }
+
         /*!
          * \brief Test for equality.
          * \todo Improve inefficient implementation.
@@ -203,16 +224,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 <class X, X V> struct typecheck {};
-            struct base { void push_back(); }; template <class C> struct derived : public C, public base {};
-            static yes hasPushHelper(...); template <class D> static no hasPushHelper(D *, typecheck<void (base::*)(), &D::push_back> * = 0);
-            template <class C> struct hasPush : public std::integral_constant<bool, sizeof(hasPushHelper((derived<C>*)0)) == sizeof(yes)> {};
-            template <class C> static iterator insertImpl(typename std::enable_if< hasPush<C>::value, C>::type &c, const T &value) { c.push_back(value); return iterator::fromImpl(c.end() - 1); }
-            template <class C> static iterator insertImpl(typename std::enable_if < !hasPush<C>::value, C >::type &c, const T &value) { return iterator::fromImpl(c.insert(value)); }
         };
 
         template <class C> class Pimpl : public PimplBase
@@ -231,13 +245,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<C>(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<const typename C::iterator *>(pos.getImpl()))); }
             //iterator erase(iterator it1, iterator it2) { return iterator::fromImpl(m_impl.erase(*static_cast<const typename C::iterator *>(it1.getImpl()), *static_cast<const typename C::iterator*>(it2.getImpl()))); }
             iterator erase(iterator it1, iterator it2) { while (it1 != it2) { it1 = iterator::fromImpl(m_impl.erase(*static_cast<const typename C::iterator *>(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<interator, bool>
+            template <class I> static I insertHelper(I i) { return i; }
+            template <class I> static I insertHelper(std::pair<I, bool> p) { return p.first; }
         };
 
         typedef QScopedPointer<PimplBase> PimplPtr;

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 <template <class> class Other>
+        Other<T> to(Other<T> other = Other<T>()) const
+        {
+            for (auto it = derived().cbegin(); it != derived().cend(); ++it) { other.push_back(*it); }
+            return other;
+        }
+
         /*!
          * \brief Return a copy containing only those elements for which a given predicate returns true.
          */
@@ -86,22 +98,20 @@ 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 <class Predicate>
         bool contains(Predicate p) const
         {
-            return std::any_of(derived().begin(), derived().end(), p);
+            return std::any_of(derived().cbegin(), derived().cend(), p);
         }
 
         /*!
-         * \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.
-         * \param object is this object in container?
-         * \return
          */
         bool contains(const T &object) const
         {
-            return std::find(derived().begin(), derived().end(), object) != derived().end();
+            return derived().find(object) != derived().cend();
         }
 
         /*!
@@ -115,6 +125,30 @@ namespace BlackMisc
             return contains(BlackMisc::Predicates::MemberEqual<T>(key1, value1));
         }
 
+        /*!
+         * \brief Remove elements for which a given predicate returns true.
+         * \pre The sequence must be initialized.
+         */
+        template <class Predicate>
+        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 <class K1, class V1>
+        void removeIf(K1 key1, V1 value1)
+        {
+            removeIf(BlackMisc::Predicates::MemberEqual<T>(key1, value1));
+        }
+
     public: // CValueObject overrides
         /*!
          * \copydoc CValueObject::toQVariant()
@@ -134,7 +168,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 += "}";
         }
@@ -161,7 +195,7 @@ namespace BlackMisc
             //const auto &o = static_cast<const CContainerBase &>(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; }
@@ -172,7 +206,7 @@ namespace BlackMisc
         virtual void marshallToDbus(QDBusArgument &argument) const
         {
             argument.beginArray(qMetaTypeId<T>());
-            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();
         }
 

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<const Pimpl&>(other).m_impl; }
                 virtual void *impl() { return &m_impl; }
             private:

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(); }
 
@@ -219,6 +219,17 @@ 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).
+         * \warning Take care that the returned non-const iterator is not compared with a const iterator.
+         */
+        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.
          */
@@ -247,28 +258,9 @@ namespace BlackMisc
             applyIf([ & ](const T &value) { return value == pattern; }, newValues);
         }
 
-        /*!
-         * \brief Remove elements for which a given predicate returns true.
-         */
-        template <class Predicate>
-        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 <class K1, class V1>
-        void removeIf(K1 key1, V1 value1)
-        {
-            removeIf(BlackMisc::Predicates::MemberEqual<T>(key1, value1));
-        }
-
         /*!
          * \brief Remove the given object, if it is contained.
+         * \pre The sequence must be initialized.
          */
         void remove(const T &object)
         {
@@ -305,6 +297,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 <class Predicate>
         void replaceOrAdd(Predicate p, const T &replacement)
@@ -315,6 +308,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 +320,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 <class K1, class V1>
         void replaceOrAdd(K1 key1, V1 value1, const T &replacement)

tests/blackmisc/testcontainers.cpp

@@ -7,8 +7,10 @@
 #include "blackmisc/collection.h"
 #include "blackmisc/sequence.h"
 #include <QList>
-#include <QString>
+#include <QVector>
+#include <QSet>
 #include <vector>
+#include <set>
 
 using namespace BlackMisc;
 
@@ -19,9 +21,9 @@ namespace BlackMiscTest
     {
         CCollection<int> c1;
         QVERIFY2(c1.empty(), "Uninitialized collection is empty");
-        auto c2 = CCollection<int>::fromImpl(QList<int>());
+        auto c2 = CCollection<int>::fromImpl(QSet<int>());
         QVERIFY2(c1 == c2, "Uninitialized and empty collections are equal");
-        c1.changeImpl(std::vector<int>());
+        c1.changeImpl(std::set<int>());
         QVERIFY2(c1 == c2, "Two empty collections are equal");
         c1.insert(1);
         QVERIFY2(c1 != c2, "Different collections are not equal");
@@ -60,7 +62,7 @@ namespace BlackMiscTest
         s1 = s2;
         QVERIFY2(s1 == s2, "Copied sequence is equal");
 
-        QVERIFY2(s1[0] = 1, "Subscripted element mutation");
+        QVERIFY2((s1[0] = 1), "Subscripted element mutation");
         QVERIFY2(s1[0] == 1, "Subscripted element has expected value");
         QVERIFY2(s1.back() == 1, "Last element has expected value");
     }