Iterator classes: removed templated ctors and added static methods to replace them,

for consistency with the container classes and to avoid any potential infinite recursion with MSVC2010.
See also commit:6a9065b

Also fixed a mistake in CSequence and CCollection, where Pimpl::erase was calling the wrong version of m_impl.erase
reported by Roland.

src/blackmisc/collection.h

@@ -238,8 +238,8 @@ namespace BlackMisc
             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 c.end() - 1; }
+            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 c.insert(value); }
+            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
@@ -249,18 +249,19 @@ namespace BlackMisc
             Pimpl(C &&c) : m_impl(std::move(c)) {}
             PimplBase *clone() const { return new Pimpl(*this); }
             PimplBase *cloneEmpty() const { return new Pimpl(C()); }
-            iterator begin() { return m_impl.begin(); }
+            iterator begin() { return iterator::fromImpl(m_impl.begin()); }
-            const_iterator begin() const { return m_impl.cbegin(); }
+            const_iterator begin() const { return const_iterator::fromImpl(m_impl.cbegin()); }
-            const_iterator cbegin() const { return m_impl.cbegin(); }
+            const_iterator cbegin() const { return const_iterator::fromImpl(m_impl.cbegin()); }
-            iterator end() { return m_impl.end(); }
+            iterator end() { return iterator::fromImpl(m_impl.end()); }
-            const_iterator end() const { return m_impl.cend(); }
+            const_iterator end() const { return const_iterator::fromImpl(m_impl.cend()); }
-            const_iterator cend() const { return m_impl.cend(); }
+            const_iterator cend() const { return const_iterator::fromImpl(m_impl.cend()); }
             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 erase(iterator pos) { return m_impl.erase(*static_cast<const typename C::iterator *>(pos.getImpl())); }
+            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 m_impl.erase(*static_cast<const typename C::iterator *>(it1.getImpl(), it2.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; }
             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;

src/blackmisc/iterator.h

@@ -44,12 +44,6 @@ namespace BlackMisc
             //! \brief Default constructor.
             ConstForwardIterator() {}
 
-            /*!
-             * \brief Constructor.
-             * \param i
-             */
-            template <class I> ConstForwardIterator(I i) : m_pimpl(new Pimpl<I>(std::move(i))) {}
-
             /*!
              * \brief Copy constructor.
              * \param other
@@ -62,13 +56,6 @@ namespace BlackMisc
              */
             ConstForwardIterator(ConstForwardIterator &&other) : m_pimpl(other.m_pimpl.take()) {}
 
-            /*!
-             * \brief Assignment.
-             * \param i
-             * \return
-             */
-            template <class I> ConstForwardIterator &operator =(I i) { m_pimpl.reset(new Pimpl<I>(std::move(i))); return *this; }
-
             /*!
              * \brief Copy assignment.
              * \param other
@@ -83,6 +70,14 @@ namespace BlackMisc
              */
             ConstForwardIterator &operator =(ConstForwardIterator &&other) { m_pimpl.reset(other.m_pimpl.take()); return *this; }
 
+            /*!
+             * \brief Create a new iterator with a specific implementation type.
+             * \tparam C Becomes the iterator's implementation type.
+             * \param c Initial value for the iterator. The value is copied.
+             * \return
+             */
+            template <class I> static ConstForwardIterator fromImpl(I i) { return ConstForwardIterator(new Pimpl<I>(std::move(i))); }
+
             /*!
              * \brief Returns a reference to the object pointed to.
              * \return
@@ -183,6 +178,8 @@ namespace BlackMisc
             typedef QScopedPointer<PimplBase> PimplPtr;
             PimplPtr m_pimpl;
 
+            explicit ConstForwardIterator(PimplBase *pimpl) : m_pimpl(pimpl) {} // private ctor used by fromImpl()
+
             // using these methods to access m_pimpl.data() eases the cognitive burden of correctly forwarding const
             PimplBase *pimpl() { return m_pimpl.data(); }
             const PimplBase *pimpl() const { return m_pimpl.data(); }
@@ -211,12 +208,6 @@ namespace BlackMisc
             //! \brief Default constructor.
             ConstBidirectionalIterator() {}
 
-            /*!
-             * \brief Constructor.
-             * \param i
-             */
-            template <class I> ConstBidirectionalIterator(I i) : m_pimpl(new Pimpl<I>(std::move(i))) {}
-
             /*!
              * \brief Copy constructor.
              * \param other
@@ -229,13 +220,6 @@ namespace BlackMisc
              */
             ConstBidirectionalIterator(ConstBidirectionalIterator &&other) : m_pimpl(other.m_pimpl.take()) {}
 
-            /*!
-             * \brief Assignment.
-             * \param i
-             * \return
-             */
-            template <class I> ConstBidirectionalIterator &operator =(I i) { m_pimpl.reset(new Pimpl<I>(std::move(i))); return *this; }
-
             /*!
              * \brief Copy assignment.
              * \param other
@@ -250,6 +234,14 @@ namespace BlackMisc
              */
             ConstBidirectionalIterator &operator =(ConstBidirectionalIterator &&other) { m_pimpl.reset(other.m_pimpl.take()); return *this; }
 
+            /*!
+             * \brief Create a new iterator with a specific implementation type.
+             * \tparam C Becomes the iterator's implementation type.
+             * \param c Initial value for the iterator. The value is copied.
+             * \return
+             */
+            template <class I> static ConstBidirectionalIterator fromImpl(I i) { return ConstBidirectionalIterator(new Pimpl<I>(std::move(i))); }
+
             /*!
              * \brief Returns a reference to the object pointed to.
              * \return
@@ -404,6 +396,8 @@ namespace BlackMisc
             typedef QScopedPointer<PimplBase> PimplPtr;
             PimplPtr m_pimpl;
 
+            explicit ConstBidirectionalIterator(PimplBase *pimpl) : m_pimpl(pimpl) {} // private ctor used by fromImpl()
+
             // using these methods to access m_pimpl.data() eases the cognitive burden of correctly forwarding const
             PimplBase *pimpl() { return m_pimpl.data(); }
             const PimplBase *pimpl() const { return m_pimpl.data(); }
@@ -432,12 +426,6 @@ namespace BlackMisc
             //! \brief Default constructor.
             BidirectionalIterator() {}
 
-            /*!
-             * \brief Constructor.
-             * \param i
-             */
-            template <class I> BidirectionalIterator(I i) : m_pimpl(new Pimpl<I>(std::move(i))) {}
-
             /*!
              * \brief Copy constructor.
              * \param other
@@ -450,13 +438,6 @@ namespace BlackMisc
              */
             BidirectionalIterator(BidirectionalIterator &&other) : m_pimpl(other.m_pimpl.take()) {}
 
-            /*!
-             * \brief Assignment.
-             * \param i
-             * \return
-             */
-            template <class I> BidirectionalIterator &operator =(I i) { m_pimpl.reset(new Pimpl<I>(std::move(i))); return *this; }
-
             /*!
              * \brief Copy assignment.
              * \param other
@@ -471,6 +452,14 @@ namespace BlackMisc
              */
             BidirectionalIterator &operator =(BidirectionalIterator &&other) { m_pimpl.reset(other.m_pimpl.take()); return *this; }
 
+            /*!
+             * \brief Create a new iterator with a specific implementation type.
+             * \tparam C Becomes the iterator's implementation type.
+             * \param c Initial value for the iterator. The value is copied.
+             * \return
+             */
+            template <class I> static BidirectionalIterator fromImpl(I i) { return BidirectionalIterator(new Pimpl<I>(std::move(i))); }
+
             /*!
              * \brief Returns a reference to the object pointed to.
              * \return
@@ -641,6 +630,8 @@ namespace BlackMisc
             typedef QScopedPointer<PimplBase> PimplPtr;
             PimplPtr m_pimpl;
 
+            explicit BidirectionalIterator(PimplBase *pimpl) : m_pimpl(pimpl) {} // private ctor used by fromImpl()
+
             // using these methods to access m_pimpl.data() eases the cognitive burden of correctly forwarding const
             PimplBase *pimpl() { return m_pimpl.data(); }
             const PimplBase *pimpl() const { return m_pimpl.data(); }

src/blackmisc/sequence.h

@@ -474,12 +474,12 @@ namespace BlackMisc
             Pimpl(C &&c) : m_impl(std::move(c)) {}
             PimplBase *clone() const { return new Pimpl(*this); }
             PimplBase *cloneEmpty() const { return new Pimpl(C()); }
-            iterator begin() { return m_impl.begin(); }
+            iterator begin() { return iterator::fromImpl(m_impl.begin()); }
-            const_iterator begin() const { return m_impl.cbegin(); }
+            const_iterator begin() const { return const_iterator::fromImpl(m_impl.cbegin()); }
-            const_iterator cbegin() const { return m_impl.cbegin(); }
+            const_iterator cbegin() const { return const_iterator::fromImpl(m_impl.cbegin()); }
-            iterator end() { return m_impl.end(); }
+            iterator end() { return iterator::fromImpl(m_impl.end()); }
-            const_iterator end() const { return m_impl.cend(); }
+            const_iterator end() const { return const_iterator::fromImpl(m_impl.cend()); }
-            const_iterator cend() const { return m_impl.cend(); }
+            const_iterator cend() const { return const_iterator::fromImpl(m_impl.cend()); }
             reference operator [](size_type index) { return m_impl[index]; }
             const_reference operator [](size_type index) const { return m_impl[index]; }
             reference front() { return m_impl.front(); }
@@ -489,11 +489,11 @@ namespace BlackMisc
             size_type size() const { return m_impl.size(); }
             bool empty() const { return m_impl.empty(); }
             void clear() { m_impl.clear(); }
-            iterator insert(iterator pos, const T &value) { return m_impl.insert(*static_cast<const typename C::iterator*>(pos.getImpl()), value); }
+            iterator insert(iterator pos, const T &value) { return iterator::fromImpl(m_impl.insert(*static_cast<const typename C::iterator*>(pos.getImpl()), value)); }
             void push_back(const T &value) { m_impl.push_back(value); }
             void pop_back() { m_impl.pop_back(); }
-            iterator erase(iterator pos) { return m_impl.erase(*static_cast<const typename C::iterator*>(pos.getImpl())); }
+            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 m_impl.erase(*static_cast<const typename C::iterator*>(it1.getImpl(), it2.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()))); }
             bool operator ==(const PimplBase &other) const { Pimpl copy = C(); for (auto i = other.cbegin(); i != other.cend(); ++i) copy.push_back(*i); return m_impl == copy.m_impl; }
         private:
             C m_impl;