refs #466 Resolved TODO items in containers.

src/blackmisc/collection.h

@@ -17,6 +17,7 @@
 #include <QScopedPointer>
 #include <algorithm>
 #include <type_traits>
+#include <typeindex>
 #include <iterator>
 #include <utility>
 #include <initializer_list>
@@ -356,13 +357,11 @@ namespace BlackMisc
 
         /*!
          * \brief Test for equality.
-         * \todo Improve inefficient implementation.
          */
-        bool operator ==(const CCollection &other) const { return (empty() && other.empty()) ? true : (size() != other.size() ? false : *pimpl() == *other.pimpl()); }
+        bool operator ==(const CCollection &other) const { return *pimpl() == *other.pimpl(); }
 
         /*!
          * \brief Test for inequality.
-         * \todo Improve inefficient implementation.
          */
         bool operator !=(const CCollection &other) const { return !(*this == other); }
 
@@ -397,6 +396,8 @@ namespace BlackMisc
             virtual const_iterator find(const T &value) const = 0;
             virtual bool operator ==(const PimplBase &other) const = 0;
             virtual void *impl() = 0;
+            virtual const void *impl() const = 0;
+            virtual std::type_index implType() const = 0;
         };
 
         template <class C> class Pimpl : public PimplBase
@@ -422,10 +423,14 @@ namespace BlackMisc
             iterator erase(iterator it1, iterator it2) override { while (it1 != it2) { it1 = iterator::fromImpl(m_impl.erase(*static_cast<const typename C::iterator *>(it1.getImpl()))); } return it1; }
             iterator find(const T &value) override { return iterator::fromImpl(m_impl.find(value)); }
             const_iterator find(const T &value) const override { return const_iterator::fromImpl(m_impl.find(value)); }
-            bool operator ==(const PimplBase &other) const override { Pimpl copy = C(); for (auto i = other.cbegin(); i != other.cend(); ++i) copy.insert(*i); return m_impl == copy.m_impl; }
+            bool operator ==(const PimplBase &other) const override { return implType() == other.implType() ? m_impl == *static_cast<const C *>(other.impl()) : size() == other.size() && std::equal(begin(), end(), other.begin()); }
             void *impl() override { return &m_impl; }
+            const void *impl() const override { return &m_impl; }
+            std::type_index implType() const override { return typeid(C); }
         private:
             C m_impl;
+            bool implEquals(const PimplBase &other) const { return m_impl == *static_cast<const C *>(other.impl()); }
+            bool equals(const PimplBase &other) const { return size() == other.size() && std::equal(begin(), end(), other.begin()); }
             // insertHelper: QOrderedSet::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; }

src/blackmisc/dictionary.h

@@ -411,18 +411,16 @@ namespace BlackMisc
         friend bool operator !=(const CDictionary &a, const CDictionary &b) { return !(a == b); }
 
         //! \copydoc BlackMisc::CValueObject::convertToQString
-        //! \todo Fix brackets
         QString convertToQString(bool i18n = false) const
         {
-            QString str = "{";
+            QString str;
             for (auto it = m_impl.cbegin(); it != m_impl.end(); ++it)
             {
                 str += "{";
                 str += CContainerHelper::stringify(it.key(), i18n) + "," + CContainerHelper::stringify(it.value(), i18n);
                 str += "}";
             }
-            if (str.isEmpty()) { str = "{"; }
-            return str += "}";
+            return "{" + str + "}";
         }
 
     public:

src/blackmisc/range.h

@@ -187,6 +187,9 @@ namespace BlackMisc
         bool isEmpty() const { return empty(); }
         //! @}
 
+        //! Returns the number of elements in the range.
+        difference_type size() const { return std::distance(begin(), end()); }
+
         //! Returns the element at the beginning of the range. Undefined if the range is empty.
         const_reference front() const { Q_ASSERT(!empty()); return *begin(); }
 

src/blackmisc/timestampobjectlist.cpp

@@ -80,24 +80,29 @@ namespace BlackMisc
     }
 
     template <class OBJ, class CONTAINER>
-    QList<CONTAINER> ITimestampObjectList<OBJ, CONTAINER>::splitByTime(qint64 msSinceEpoch, bool alreadySortedLatestFirst) const
+    QList<CONTAINER> ITimestampObjectList<OBJ, CONTAINER>::splitByTime(qint64 msSinceEpoch, bool sortedLatestFirst) const
     {
-        // fixme: Split by time is one of the most frequently called functions in interpolator, so any performance improvement here counts
-        CONTAINER newer(this->container());
-        if (!alreadySortedLatestFirst) { newer.sortLatestFirst(); }
-        CONTAINER older;
-        for (auto it = newer.begin(); it != newer.end(); ++it)
+        QList<CONTAINER> result { {}, {} };
+        const auto &c = this->container();
+        if (sortedLatestFirst)
         {
-            if (it->isOlderThan(msSinceEpoch))
+            // O(log n) comparisons and O(n) copies
+            struct Comparator
             {
-                older.insert(CRange<typename CONTAINER::iterator>(it, newer.end()));
-                newer.erase(it, newer.end());
-                break;
-            }
+                bool operator()(const OBJ &a, qint64 b) const { return a.isNewerThan(b); }
+                bool operator()(qint64 a, const OBJ &b) const { return b.isOlderThan(a); }
+            };
+            auto it = std::upper_bound(c.begin(), c.end(), msSinceEpoch, Comparator());
+            std::copy(c.begin(), it, std::back_inserter(result[0]));
+            std::copy(it, c.end(), std::back_inserter(result[1]));
         }
-
-        // before / after
-        return QList<CONTAINER>({newer, older});
+        else
+        {
+            // O(n) comparisons and O(n) copies
+            std::partition_copy(c.begin(), c.end(), std::back_inserter(result[0]), std::back_inserter(result[1]),
+                [msSinceEpoch](const OBJ & obj) { return ! obj.isNewerThan(msSinceEpoch); });
+        }
+        return result;
     }
 
     template <class OBJ, class CONTAINER>

src/blackmisc/timestampobjectlist.h

@@ -44,9 +44,9 @@ namespace BlackMisc
         //! List of objects after msSinceEpoch
         CONTAINER findAfter(qint64 msSinceEpoch) const;
 
-        //! Split into 2 containers, [0] >= msSinceEpoch ("newer") [b] < msSinceEpoch ("older")
-        //! \note Sort order: latest elements first
-        QList<CONTAINER> splitByTime(qint64 msSinceEpoch, bool alreadySortedLatestFirst = false) const;
+        //! Partition into two containers, first [0,msSinceEpoch] and second (msSinceEpoch,LLONG_MAX].
+        //! Within each of the two parts, the original relative ordering of the elements is preserved.
+        QList<CONTAINER> splitByTime(qint64 msSinceEpoch, bool sortedLatestFirst = false) const;
 
         //! Latest value
         OBJ latestValue() const;