Cannot get BOOST_FOREACH to work with my custom class

Question

I implemented a trivial class MyClass that has an array allocated with new inside it (I know that I could use a STL container, but I'm trying to understand how they work). I also created an iterator subclass, able to iterate on all the elements of a MyClass object:

class MyIterator : public iterator<forward_iterator_tag,int>
{
private:
    int* data= nullptr;
    int length;
    int pointer=0;
    int nullvalue=0;
public:
    MyIterator(int* data, int length, bool end= false)
    {
        this->data= data;
        this->length= length;
        if(end)
            pointer=-1;
    }
    ~MyIterator()
    {
        delete[] data;
    }
    MyIterator& operator++()
    {
        if(pointer!= length-1)
        {
            pointer++;
        }
        else
        {
            pointer= -1;
        }
        return *this;
    }
    bool operator==(const MyIterator& other)
    {
        return pointer==other.pointer;
    }
    bool operator!=(const MyIterator& other)
    {
        return pointer!= other.pointer;
    }
    int& operator* ()
    {
        if(pointer==-1)
            return nullvalue;
        else
            return data[pointer];
    }
};

class MyClass
{
private:
    int* data= nullptr;
    int length= 100;
public:
    MyClass()
    {
        data= new int[length];
        for(int i=0; i<length;i++)
            data[i]=i+1;
    }
    iterator<forward_iterator_tag,int> begin()
    {
        return MyIterator(data,length);
    }
    iterator<forward_iterator_tag,int> end()
    {
        return MyIterator(data,length,true);
    }
};

While the iterator works if I use it this way:

for(MyIterator i= MyClass_instance.begin(); i!=MyClass_instance.end();i++) {...}

It doesn't work if I try to use it with BOOST_FOREACH:

BOOST_FOREACH(int i, MyClass_instance) {...}

These are the errors that I get:

enter image description here

score 3 · Accepted Answer · edited May 23 '17 at 12:05

You're slicing your iterator by returning them as std::iterator<> by value. You cannot do that.

Returning by reference would avoid the slicing problem but introduces a worse problem: it returns a reference to a temporary¹.

Hence, fix it by returning your actual iterator type by value.
Your type was missing a const iterator.
All the iterator members weren't const-correct.

Also, according to the page Extensibility it looks like you need to add

namespace boost {
    template<> struct range_mutable_iterator<MyClass> {
        typedef MyClass::MyIterator type;
    };

    template<> struct range_const_iterator<MyClass> {
        typedef MyClass::MyConstIterator type;
    };
}

There is serious trouble with the Rule-Of-Three implementation for your iterator type (What is The Rule of Three?).

You're deleting the container's data every time an iterator goes out of existence. And MyClass itself never frees the data...

Fixing most (?) of the above:

Live On Coliru

#include <iterator>
#include <boost/foreach.hpp>

class MyClass
{
private:
    int* data  = nullptr;
    int length = 100;

public:
    class MyIterator : public std::iterator<std::forward_iterator_tag, int>
    {
        public:
            MyIterator(int* data, int length, bool end = false)
            {
                this->data= data;
                this->length= length;
                if(end)
                    pointer=-1;
            }
            MyIterator& operator++()
            {
                if(pointer!= length-1) {
                    pointer++;
                }
                else {
                    pointer= -1;
                }
                return *this;
            }

            bool operator==(const MyIterator& other) const { return pointer==other.pointer; }
            bool operator!=(const MyIterator& other) const { return pointer!= other.pointer; }
            int& operator*() const
            {
                if(pointer==-1)
                    return nullvalue;
                else
                    return data[pointer];
            }
        private:
            value_type* data     = nullptr;
            int length;
            int pointer          = 0;
            mutable value_type nullvalue = 0;
    };

    class MyConstIterator : public std::iterator<std::forward_iterator_tag, const int>
    {
        public:
            MyConstIterator(int const* data, int length, bool end = false)
            {
                this->data= data;
                this->length= length;
                if(end)
                    pointer=-1;
            }
            MyConstIterator& operator++()
            {
                if(pointer!= length-1) {
                    pointer++;
                }
                else {
                    pointer= -1;
                }
                return *this;
            }

            bool operator==(const MyConstIterator& other) const { return pointer==other.pointer; }
            bool operator!=(const MyConstIterator& other) const { return pointer!= other.pointer; }
            int const& operator*() const
            {
                if(pointer==-1)
                    return nullvalue;
                else
                    return data[pointer];
            }
        private:
            value_type* data     = nullptr;
            int length;
            int pointer          = 0;
            value_type nullvalue = 0;
    };

public:
    typedef MyIterator iterator_type;
    typedef MyConstIterator const_iterator_type;

    MyClass()
    {
        data= new int[length];
        for(int i=0; i<length;i++)
            data[i]=i+1;
    }
    ~MyClass() {
        delete[] data;
    }
    iterator_type begin()             { return MyIterator(data,length);      }
    iterator_type end()               { return MyIterator(data,length,true); }
    const_iterator_type begin() const { return MyConstIterator(data,length);      }
    const_iterator_type end()   const { return MyConstIterator(data,length,true); }
};

namespace boost {
    template<> struct range_mutable_iterator<MyClass> {
        typedef MyClass::MyIterator type;
    };

    template<> struct range_const_iterator<MyClass> {
        typedef MyClass::MyConstIterator type;
    };
}

#include <iostream>

int main()
{
    MyClass c;
    BOOST_FOREACH(int i, c) {
        std::cout << i << "\n";
    }
}

¹ (unless you store the iterators somewhere else, but that would be a huge anti-pattern for many many reasons)

working sample **[Live On Coliru](http://coliru.stacked-crooked.com/a/5aded9c74b968671)** — sehe, Dec 21 '14 at 21:54
I get an incomprehensible syntax error: "No matching function for call to 'deref'", in the same line where you call BOOST_FOREACH. The problem is inside a struct named struct __iterator_traits_impl<_Iter, true>, inside the STL's iterator header. — Ramy Al Zuhouri, Dec 21 '14 at 22:54
@RamyAlZuhouri interesting. Did you notice the live sample live works? I'd be tempted to assume you're doing something different. I can't really reason about the code you're not showing. — sehe, Dec 21 '14 at 23:13

Cannot get BOOST_FOREACH to work with my custom class

1 Answers1