diff --git a/src/blackmisc/collection.h b/src/blackmisc/collection.h index f04645ce2..0eafccb2c 100644 --- a/src/blackmisc/collection.h +++ b/src/blackmisc/collection.h @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -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 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(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(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(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 template static I insertHelper(I i) { return i; } template static I insertHelper(std::pair p) { return p.first; } diff --git a/src/blackmisc/dictionary.h b/src/blackmisc/dictionary.h index feea2bb84..384a0807b 100644 --- a/src/blackmisc/dictionary.h +++ b/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: diff --git a/src/blackmisc/range.h b/src/blackmisc/range.h index ce2ce60a2..3d5da49eb 100644 --- a/src/blackmisc/range.h +++ b/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(); } diff --git a/src/blackmisc/timestampobjectlist.cpp b/src/blackmisc/timestampobjectlist.cpp index 81062a763..523abfa74 100644 --- a/src/blackmisc/timestampobjectlist.cpp +++ b/src/blackmisc/timestampobjectlist.cpp @@ -80,24 +80,29 @@ namespace BlackMisc } template - QList ITimestampObjectList::splitByTime(qint64 msSinceEpoch, bool alreadySortedLatestFirst) const + QList ITimestampObjectList::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 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(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({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 diff --git a/src/blackmisc/timestampobjectlist.h b/src/blackmisc/timestampobjectlist.h index f799fc517..1d46caabf 100644 --- a/src/blackmisc/timestampobjectlist.h +++ b/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 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 splitByTime(qint64 msSinceEpoch, bool sortedLatestFirst = false) const; //! Latest value OBJ latestValue() const; diff --git a/tests/blackmisc/testcontainers.cpp b/tests/blackmisc/testcontainers.cpp index 205c4634c..906ae3c86 100644 --- a/tests/blackmisc/testcontainers.cpp +++ b/tests/blackmisc/testcontainers.cpp @@ -235,23 +235,23 @@ namespace BlackMiscTest QVERIFY2(ms == ts, "Latest value not first"); // split in half - situations.sortOldestFirst(); // check that we really get latest first - QList split = situations.splitByTime(ts - ((no / 2) * 10) + 1); - CAircraftSituationList before = split[0]; - CAircraftSituationList after = split[1]; - - int beforeSize = before.size(); - int afterSize = after.size(); - - QVERIFY(beforeSize == no / 2); - QVERIFY(afterSize == no / 2); - - // check sort order, latest should - for (int i = 0; i < no; ++i) + for (bool sortLatestFirst : { false, true }) { - CAircraftSituation s = (i < no / 2) ? before[i] : after[i - no / 2]; - ms = s.getMSecsSinceEpoch(); - QVERIFY2(ms == ts - 10 * i, "time does not match"); + sortLatestFirst ? situations.sortLatestFirst() : situations.sortOldestFirst(); + qint64 splitTime = ts - ((no / 2) * 10) + 1; + QList split = situations.splitByTime(splitTime); + CAircraftSituationList before = split[0]; + CAircraftSituationList after = split[1]; + + int beforeSize = before.size(); + int afterSize = after.size(); + QVERIFY(beforeSize == no / 2); + QVERIFY(afterSize == no / 2); + + // check partitioning + auto isNewer = [splitTime](const CAircraftSituation &as) { return as.isNewerThan(splitTime); }; + QVERIFY2(std::none_of(before.cbegin(), before.cend(), isNewer), "before contains a time which is after"); + QVERIFY2(std::all_of(after.cbegin(), after.cend(), isNewer), "after contains a time which is before"); } // test shifting