From 921ef30edab965866d62259d2b1840c42a630c34 Mon Sep 17 00:00:00 2001
From: Mathew Sutcliffe <oktal3700@gmail.com>
Date: Thu, 23 Nov 2017 21:59:08 +0000
Subject: [PATCH] Use C++14 features to simplify our container base classes

Summary: Using C++14 function return type deduction we can simplify the CRTP in CContainerBase etc.

Reviewers: #swift_developers, rwinklmeier

Reviewed By: #swift_developers, rwinklmeier

Subscribers: jenkins

Tags: #swift_pilot_client

Differential Revision: https://dev.swift-project.org/D65
---
 src/blackmisc/collection.h    |  2 +-
 src/blackmisc/containerbase.h | 50 ++++++++++++++++++++---------------
 src/blackmisc/range.h         | 50 +++++++++++++++++------------------
 src/blackmisc/sequence.h      |  2 +-
 src/blackmisc/statusmessage.h |  8 +++---
 5 files changed, 58 insertions(+), 54 deletions(-)

diff --git a/src/blackmisc/collection.h b/src/blackmisc/collection.h
index d6d066a33..b066a0669 100644
--- a/src/blackmisc/collection.h
+++ b/src/blackmisc/collection.h
@@ -67,7 +67,7 @@ namespace BlackMisc
      */
     template <class T>
     class CCollection :
-        public CContainerBase<CCollection, T, Iterators::ConstForwardIterator<T>>,
+        public CContainerBase<CCollection<T>>,
         public Mixin::Icon<CCollection<T>>
     {
     public:
diff --git a/src/blackmisc/containerbase.h b/src/blackmisc/containerbase.h
index 38a57ea22..cb42fe0e4 100644
--- a/src/blackmisc/containerbase.h
+++ b/src/blackmisc/containerbase.h
@@ -49,18 +49,17 @@ namespace BlackMisc
     /*!
      * Base class for CCollection and CSequence adding mutating operations and CValueObject facility on top of CRangeBase.
      */
-    template <template <class> class C, class T, class CIt>
+    template <class Derived>
     class CContainerBase :
-        public CRangeBase<C<T>, CIt>,
-        public Mixin::MetaType<C<T>>,
-        public Mixin::DBusOperators<C<T>>,
-        public Mixin::JsonOperators<C<T>>,
-        public Mixin::String<C<T>>
+        public CRangeBase<Derived>,
+        public Mixin::MetaType<Derived>,
+        public Mixin::DBusOperators<Derived>,
+        public Mixin::JsonOperators<Derived>,
+        public Mixin::String<Derived>
     {
     public:
-
         //! \copydoc BlackMisc::CValueObject::compare
-        friend int compare(const C<T> &a, const C<T> &b)
+        friend int compare(const Derived &a, const Derived &b)
         {
             for (auto i = a.cbegin(), j = b.cbegin(); i != a.cend() && j != b.cend(); ++i, ++j)
             {
@@ -75,13 +74,21 @@ namespace BlackMisc
 
         //! 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
+        auto to() const
+        {
+            return to(Other<typename Derived::value_type>());
+        }
+        //! \tparam T element type of the new container.
+        //! \param other an initial value for the new container; will be copied.
+        template <template <class> class Other, class T>
+        Other<T> to(Other<T> other) const
         {
             for (auto it = derived().cbegin(); it != derived().cend(); ++it) { other.push_back(*it); }
             return other;
         }
+        //! @}
 
         //! Remove elements matching some particular key/value pair(s).
         //! \param k0 A pointer to a member function of T.
@@ -96,7 +103,7 @@ namespace BlackMisc
 
     public:
         //! Simplifies composition, returns 0 for performance
-        friend uint qHash(const C<T> &) { return 0; }
+        friend uint qHash(const Derived &) { return 0; }
 
         //! \copydoc BlackMisc::Mixin::JsonByMetaClass::toJson
         QJsonObject toJson() const
@@ -131,7 +138,7 @@ namespace BlackMisc
                 CJsonScope scope("containerbase", index++);
                 Q_UNUSED(scope);
                 QJsonValueRef ref = (*i);
-                T val;
+                typename Derived::value_type val;
                 ref >> val;
                 derived().insert(std::move(val));
             }
@@ -159,24 +166,23 @@ namespace BlackMisc
         }
 
         //! To string list
-        QStringList toStringList(bool i18n = false) const {
+        QStringList toStringList(bool i18n = false) const
+        {
             QStringList sl;
-            for (const T &obj : this->derived()) {
-                sl.append(obj.toQString(i18n));
-            }
+            for (const auto &obj : this->derived()) { sl.append(obj.toQString(i18n)); }
             return sl;
         }
 
     protected:
         //! \copydoc BlackMisc::CValueObject::getMetaTypeId
-        int getMetaTypeId() const { return qMetaTypeId<C<T>>(); }
+        int getMetaTypeId() const { return qMetaTypeId<Derived>(); }
 
     public:
         //! \copydoc BlackMisc::CValueObject::marshallToDbus
         void marshallToDbus(QDBusArgument &argument) const
         {
-            argument.beginArray(qMetaTypeId<T>());
-            std::for_each(derived().cbegin(), derived().cend(), [ & ](const T & value) { argument << value; });
+            argument.beginArray(qMetaTypeId<typename Derived::value_type>());
+            std::for_each(derived().cbegin(), derived().cend(), [ & ](const auto & value) { argument << value; });
             argument.endArray();
         }
 
@@ -185,13 +191,13 @@ namespace BlackMisc
         {
             derived().clear();
             argument.beginArray();
-            while (!argument.atEnd()) { T value; argument >> value; derived().insert(value); }
+            while (!argument.atEnd()) { typename Derived::value_type value; argument >> value; derived().insert(value); }
             argument.endArray();
         }
 
     private:
-        C<T> &derived() { return static_cast<C<T> &>(*this); }
-        const C<T> &derived() const { return static_cast<const C<T> &>(*this); }
+        Derived &derived() { return static_cast<Derived &>(*this); }
+        const Derived &derived() const { return static_cast<const Derived &>(*this); }
     };
 
 }
diff --git a/src/blackmisc/range.h b/src/blackmisc/range.h
index e3716ea39..2147a72f4 100644
--- a/src/blackmisc/range.h
+++ b/src/blackmisc/range.h
@@ -44,48 +44,49 @@ namespace BlackMisc
      * Any container class with begin and end iterators can inherit from this CRTP class
      * to gain some useful algorithms as member functions.
      * \tparam Derived The most derived container class inheriting from this instantiation.
-     * \tparam CIt the const_iterator of Derived.
      */
-    template <class Derived, class CIt>
+    template <class Derived>
     class CRangeBase
     {
-        using value_type = typename std::iterator_traits<CIt>::value_type;
-        using const_reference = typename std::iterator_traits<CIt>::reference;
-
     public:
         //! Return a new container generated by applying some transformation function to all elements of this one.
         template <class F>
-        inline auto transform(F function) const
-            -> CRange<Iterators::TransformIterator<CIt, F>>;
+        inline auto transform(F function) const;
 
         //! Return a copy containing only those elements for which a given predicate returns true.
         template <class Predicate>
-        inline auto findBy(Predicate p) const
-            -> CRange<Iterators::ConditionalIterator<CIt, Predicate>>;
+        inline auto findBy(Predicate p) const;
 
         //! Return a copy containing only those elements matching some particular key/value pair(s).
         //! \param k0 A pointer to a member function of T.
         //! \param v0 A value to compare against the value returned by k0.
         //! \param keysValues Zero or more additional pairs of { pointer to member function of T, value to compare it against }.
         template <class K0, class V0, class... KeysValues>
-        inline auto findBy(K0 k0, V0 v0, KeysValues... keysValues) const
-            -> CRange<Iterators::ConditionalIterator<CIt, decltype(BlackMisc::Predicates::MemberEqual(k0, v0, keysValues...))>>;
+        inline auto findBy(K0 k0, V0 v0, KeysValues... keysValues) const;
 
         //! Return a reference to the first element for which a given predicate returns true. Undefined if there is none.
         template <class Predicate>
-        const_reference findFirstBy(Predicate p) const { return findBy(p).front(); }
+        const auto &findFirstBy(Predicate p) const { return findBy(p).front(); }
 
         //! Return a reference to the first element matching some particular key/value pair(s). Undefined if there is none.
         template <class K, class V>
-        const_reference findFirstBy(K key, V value) const { return findBy(key, value).front(); }
+        const auto &findFirstBy(K key, V value) const { return findBy(key, value).front(); }
+
+        //! Return a copy of the first element for which a given predicate returns true, or a default value if there is none.
+        template <class Predicate, class Value>
+        auto findFirstByOrDefault(Predicate p, const Value &def) const { return findBy(p).frontOrDefault(def); }
 
         //! Return a copy of the first element for which a given predicate returns true, or a default value if there is none.
         template <class Predicate>
-        value_type findFirstByOrDefault(Predicate p, const value_type &def = value_type{}) const { return findBy(p).frontOrDefault(def); }
+        auto findFirstByOrDefault(Predicate p) const { return findBy(p).frontOrDefault(); }
 
         //! Return a copy of the first element matching some particular key/value pair(s), or a default value if there is none.
-        template <class K, class V>
-        value_type findFirstByOrDefault(K key, V value, const value_type &def = value_type{}) const { return findBy(key, value).frontOrDefault(def); }
+        template <class K, class V, class Value>
+        auto findFirstByOrDefault(K key, V value, const Value &def) const { return findBy(key, value).frontOrDefault(def); }
+
+        //! Return a copy of the first element matching some particular key/value pair(s), or a default value if there is none.
+        template <class T, class K, class V>
+        auto findFirstByOrDefault(K T::* key, V value) const { return findBy(key, value).frontOrDefault(); }
 
         //! Return true if there is an element for which a given predicate returns true.
         template <class Predicate>
@@ -167,7 +168,7 @@ namespace BlackMisc
      *          during the lifetime of the range.
      */
     template <class I>
-    class CRange : public CRangeBase<CRange<I>, I>
+    class CRange : public CRangeBase<CRange<I>>
     {
     public:
         //! STL compatibility
@@ -340,26 +341,23 @@ namespace BlackMisc
     /*
      * Member functions of CRangeBase template defined out of line, because they depend on CRange etc.
      */
-    template <class Derived, class CIt>
+    template <class Derived>
     template <class F>
-    auto CRangeBase<Derived, CIt>::transform(F function) const
-        -> CRange<Iterators::TransformIterator<CIt, F>>
+    auto CRangeBase<Derived>::transform(F function) const
     {
         return makeRange(Iterators::makeTransformIterator(derived().cbegin(), function), derived().cend());
     }
 
-    template <class Derived, class CIt>
+    template <class Derived>
     template <class Predicate>
-    auto CRangeBase<Derived, CIt>::findBy(Predicate p) const
-        -> CRange<Iterators::ConditionalIterator<CIt, Predicate>>
+    auto CRangeBase<Derived>::findBy(Predicate p) const
     {
         return makeRange(Iterators::makeConditionalIterator(derived().cbegin(), derived().cend(), p), derived().cend());
     }
 
-    template <class Derived, class CIt>
+    template <class Derived>
     template <class K0, class V0, class... KeysValues>
-    auto CRangeBase<Derived, CIt>::findBy(K0 k0, V0 v0, KeysValues... keysValues) const
-        -> CRange<Iterators::ConditionalIterator<CIt, decltype(BlackMisc::Predicates::MemberEqual(k0, v0, keysValues...))>>
+    auto CRangeBase<Derived>::findBy(K0 k0, V0 v0, KeysValues... keysValues) const
     {
         return findBy(BlackMisc::Predicates::MemberEqual(k0, v0, keysValues...));
     }
diff --git a/src/blackmisc/sequence.h b/src/blackmisc/sequence.h
index a81f42a75..9fe67d416 100644
--- a/src/blackmisc/sequence.h
+++ b/src/blackmisc/sequence.h
@@ -43,7 +43,7 @@ namespace BlackMisc
      */
     template <class T>
     class CSequence :
-        public CContainerBase<CSequence, T, Iterators::ConstRandomAccessIterator<T>>,
+        public CContainerBase<CSequence<T>>,
         public Mixin::Icon<CSequence<T>>
     {
     public:
diff --git a/src/blackmisc/statusmessage.h b/src/blackmisc/statusmessage.h
index d6c4bcb60..4e9914e82 100644
--- a/src/blackmisc/statusmessage.h
+++ b/src/blackmisc/statusmessage.h
@@ -367,8 +367,8 @@ namespace BlackMisc
     };
 
     // CContainerBase methods implemented out-of-line to avoid circular include
-    template <template <class> class C, class T, class CIt>
-    CStatusMessage CContainerBase<C, T, CIt>::convertFromJsonNoThrow(const QJsonObject &json, const CLogCategoryList &categories, const QString &prefix)
+    template <class Derived>
+    CStatusMessage CContainerBase<Derived>::convertFromJsonNoThrow(const QJsonObject &json, const CLogCategoryList &categories, const QString &prefix)
     {
         try
         {
@@ -382,8 +382,8 @@ namespace BlackMisc
     }
 
     //! Call convertFromJson, catch any CJsonException that is thrown and return it as CStatusMessage.
-    template <template <class> class C, class T, class CIt>
-    CStatusMessage CContainerBase<C, T, CIt>::convertFromJsonNoThrow(const QString &jsonString, const CLogCategoryList &categories, const QString &prefix)
+    template <class Derived>
+    CStatusMessage CContainerBase<Derived>::convertFromJsonNoThrow(const QString &jsonString, const CLogCategoryList &categories, const QString &prefix)
     {
         try
         {