Completing ChannelList Iterator API (#2384)

* Added additional operator overloads '--', '*' and '->' for ChannelList::Iterator and ChannelList::ConstIterator. Also added global operator overloads '==' and '!=' for Iterator class.
(issue #1325)
Signed-off-by: Aries Moczar <arcmantis@protulae.com>

* Added '==' and '!=' overloads for mixed Iterator and ConstIterator comparisons. Added Iterator and ConstIterator traits.

Signed-off-by: Aries Moczar <arcmantis@protulae.com>

* Added test to testChannels unit-test to verify functionality of ChannelList::Iterator/ConstIterator

Signed-off-by: Aries Moczar <arcmantis@protulae.com>

---------

Signed-off-by: Aries Moczar <arcmantis@protulae.com>
Co-authored-by: Cary Phillips <cary@ilm.com>

Author: Arcmantis18

src/lib/OpenEXR/ImfChannelList.h

@@ -254,6 +254,12 @@ class IMF_EXPORT_TYPE ChannelList
 class IMF_EXPORT_TYPE ChannelList::Iterator
 {
 public:
+    using iterator_category = std::bidirectional_iterator_tag;
+    using value_type          = Channel;
+    using difference_type     = std::ptrdiff_t;
+    using pointer             = Channel*;
+    using reference           = Channel&;
+
     IMF_EXPORT
     Iterator ();
     IMF_EXPORT
@@ -264,20 +270,45 @@ class IMF_EXPORT_TYPE ChannelList::Iterator
     IMF_EXPORT
     Iterator operator++ (int);
 
+    IMF_EXPORT
+    Iterator& operator-- ();
+    IMF_EXPORT
+    Iterator operator-- (int);
+
+    IMF_EXPORT
+    reference operator* () const;
+
+    IMF_EXPORT
+    pointer operator-> () const;
+
     IMF_EXPORT
     const char* name () const;
     IMF_EXPORT
-    Channel& channel () const;
+    reference channel () const;
 
 private:
     friend class ChannelList::ConstIterator;
 
+    friend bool operator== (Iterator&, Iterator&);
+    friend bool operator== (Iterator&, const ConstIterator&);
+    friend bool operator== (const ConstIterator&, Iterator&);
+
+    friend bool operator!= (Iterator&, Iterator&);
+    friend bool operator!= (Iterator&, const ConstIterator&);
+    friend bool operator!= (const ConstIterator&, Iterator&);
+
     ChannelList::ChannelMap::iterator _i;
 };
 
 class IMF_EXPORT_TYPE ChannelList::ConstIterator
 {
 public:
+    using iterator_category = std::bidirectional_iterator_tag;
+    using value_type          = Channel;
+    using difference_type     = std::ptrdiff_t;
+    using pointer             = const Channel*;
+    using reference           = const Channel&;
+
     IMF_EXPORT
     ConstIterator ();
     IMF_EXPORT
@@ -290,14 +321,30 @@ class IMF_EXPORT_TYPE ChannelList::ConstIterator
     IMF_EXPORT
     ConstIterator operator++ (int);
 
+    IMF_EXPORT
+    ConstIterator& operator-- ();
+    IMF_EXPORT
+    ConstIterator operator-- (int);
+
+    IMF_EXPORT
+    reference operator* () const;
+
+    IMF_EXPORT
+    pointer operator-> () const;
+
     IMF_EXPORT
     const char* name () const;
     IMF_EXPORT
-    const Channel& channel () const;
+    reference channel () const;
 
 private:
     friend bool operator== (const ConstIterator&, const ConstIterator&);
+    friend bool operator== (Iterator&, const ConstIterator&);
+    friend bool operator== (const ConstIterator&, Iterator&);
+
     friend bool operator!= (const ConstIterator&, const ConstIterator&);
+    friend bool operator!= (Iterator&, const ConstIterator&);
+    friend bool operator!= (const ConstIterator&, Iterator&);
 
     ChannelList::ChannelMap::const_iterator _i;
 };
@@ -333,13 +380,40 @@ ChannelList::Iterator::operator++ (int)
     return tmp;
 }
 
+inline ChannelList::Iterator&
+ChannelList::Iterator::operator-- ()
+{
+    --_i;
+    return *this;
+}
+
+inline ChannelList::Iterator
+ChannelList::Iterator::operator-- (int)
+{
+    Iterator tmp = *this;
+    --_i;
+    return tmp;
+}
+
+inline ChannelList::Iterator::reference
+ChannelList::Iterator::operator* () const
+{
+    return _i->second;
+}
+
+inline ChannelList::Iterator::pointer
+ChannelList::Iterator::operator-> () const
+{
+    return &_i->second;
+}
+
 inline const char*
 ChannelList::Iterator::name () const
 {
     return *_i->first;
 }
 
-inline Channel&
+inline ChannelList::Iterator::reference
 ChannelList::Iterator::channel () const
 {
     return _i->second;
@@ -379,30 +453,99 @@ ChannelList::ConstIterator::operator++ (int)
     return tmp;
 }
 
+inline ChannelList::ConstIterator&
+ChannelList::ConstIterator::operator-- ()
+{
+    --_i;
+    return *this;
+}
+
+inline ChannelList::ConstIterator
+ChannelList::ConstIterator::operator-- (int)
+{
+    ConstIterator tmp = *this;
+    --_i;
+    return tmp;
+}
+
+inline ChannelList::ConstIterator::reference
+ChannelList::ConstIterator::operator* () const
+{
+    return _i->second;
+}
+
+inline ChannelList::ConstIterator::pointer
+ChannelList::ConstIterator::operator-> () const
+{
+    return &_i->second;
+}
+
 inline const char*
 ChannelList::ConstIterator::name () const
 {
     return *_i->first;
 }
 
-inline const Channel&
+inline ChannelList::ConstIterator::reference
 ChannelList::ConstIterator::channel () const
 {
     return _i->second;
 }
 
+inline bool
+operator== (
+    ChannelList::Iterator& x, ChannelList::Iterator& y)
+{
+    return x._i == y._i;
+}
+
 inline bool
 operator== (
     const ChannelList::ConstIterator& x, const ChannelList::ConstIterator& y)
 {
     return x._i == y._i;
 }
 
+inline bool
+operator== (
+    ChannelList::Iterator& x, const ChannelList::ConstIterator& y)
+{
+    return x._i == y._i;
+}
+
+inline bool
+operator== (
+    const ChannelList::ConstIterator& x, ChannelList::Iterator& y)
+{
+    return x._i == y._i;
+}
+
+inline bool
+operator!= (
+    ChannelList::Iterator& x, ChannelList::Iterator& y)
+{
+    return !(x._i == y._i);
+}
+
 inline bool
 operator!= (
     const ChannelList::ConstIterator& x, const ChannelList::ConstIterator& y)
 {
-    return !(x == y);
+    return !(x._i == y._i);
+}
+
+inline bool
+operator!= (
+    ChannelList::Iterator& x, const ChannelList::ConstIterator& y)
+{
+    return !(x._i == y._i);
+}
+
+inline bool
+operator!= (
+    const ChannelList::ConstIterator& x, ChannelList::Iterator& y)
+{
+    return !(x._i == y._i);
 }
 
 OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_EXIT

src/test/OpenEXRTest/testChannels.cpp

@@ -218,13 +218,82 @@ writeRead (
     cout << endl;
 }
 
+void
+verifyIterators(void)
+{
+    ChannelList l1;
+
+    l1.insert("A", Channel(HALF));
+    l1.insert("B", Channel(HALF));
+    l1.insert("C", Channel(HALF));
+
+    ChannelList::Iterator it1 = l1.begin();
+    ChannelList::ConstIterator cit1 = l1.begin();
+
+    ChannelList::Iterator it2 = l1.end();
+    ChannelList::ConstIterator cit2 = l1.end();
+
+    // Test global '!=' overloads
+    assert(it1 != it2);
+    assert(it1 != cit2);
+    assert(cit1 != it2);
+    assert(cit1 != cit2);
+
+    // Test global '==' overloads
+    assert(it1 == it1);
+    assert(it1 == cit1);
+    assert(cit1 == it1);
+    assert(cit1 == cit1);
+
+    // Test post/pre '++' overloads
+    it1++;
+    ++it1;
+    cit1++;
+    ++cit1;
+    
+    // Test post/pre '--' overloads
+    it1--;
+    --it1;
+    cit1--;
+    --cit1;
+
+    assert(it1 == l1.begin());
+    assert(cit1 == l1.begin());
+     
+    while (it1 != l1.end())
+    {
+        
+        // Test '*' overloads
+        Channel& c = *it1;
+        const Channel& cc = *cit1;
+        assert(c.type == HALF);
+        assert(cc.type == HALF);
+        
+        // Test '->' overload
+        assert(it1->type == HALF);
+        assert(cit1->type == HALF);
+
+        // Test pre/post '++' overloads
+        it1++;
+        cit1++;
+
+    }
+
+    assert(it1 == l1.end());
+    assert(cit1 == l1.end());
+}
+
 } // namespace
 
 void
 testChannels (const std::string& tempDir)
 {
     try
     {
+        cout << "Testing ChannelList Iterator behavior" << endl;
+
+        verifyIterators ();
+
         cout << "Testing filling of missing channels" << endl;
 
         const int W = 117;