[boost] Re: Re: Advice on using iterator_adaptor

From: Keith MacDonald (boost_at_[hidden])
Date: 2003-10-01 04:52:47

Hi Thomas,

Thanks for your constructive criticism.  I've now changed the iterators,
following your advice, and it looks a much neater solution, if only I could
make it work :-(

I've got a simple loop, like this:

 container3 c3;
 for (container3::const_iterator it = c3.begin(),
  it != c3.end();
  ++it)
 { ... }

For reasons I don't understand, the compiler resolves c3.end() to a
container3::iterator, for the not equal test, and then spews out a "could
not deduce template argument" error, for the member function equal, which is
too long to include here.  I've zipped it up, along with the header and test
source files, and uploaded it here:

 ftp://69.57.152.66/pub/tmp/itertest.zip (2.2KB)

If you have the time, please will you have a quick look to see what I've
done wrong.

Thanks,
Keith

"Thomas Witt" <witt_at_[hidden]> wrote in message
news:3F79CE70.8030303_at_acm.org...
>
> Keith,
>
> Keith MacDonald wrote:
> > Hi Thomas,
> >
> > I couldn't have begun to tackle the problem without your advice, but
even
> > then I battled with obscure compiler error messages for a long time,
before
> > I got it working.  Would you mind taking a critical look at the finished
> > class (below), to make sure I got it right?
>
> Almost ;-).
>
> >
> > One query I have about the implementation of iterator_facade is why the
> > iterator -> const_iterator conversion constructors, and the equal and
> > distance_to members take value, rather than reference parameters.
There's
> > no penalty for basic types, but there is for structs.
>
> I use to use non-reference parameters for types where copying is cheap.
> Using reference parameters does not do any harm here.
>
> >
> > Thanks,
> > Keith MacDonald
> >
> >
> > [code]
> >
> > // cont3.h
> >
> > #include <boost/iterator/iterator_facade.hpp>
> >
> > class container3
> > {
> > public:
> >     // Iteration:
> >     class iterator;
> >     class const_iterator;
> >
> >     // Read-only iterator:
> >     typedef boost::iterator_facade< const_iterator,
> >                                     unsigned char const,
> >                                     boost::readable_lvalue_iterator_tag,
> >                                     boost::random_access_traversal_tag
> >                                 > boost_const_iterator;
> >
> >     class const_iterator
> >         : public boost_const_iterator
> >     {
> >     public:
> >
> >         const_iterator()
> >             { m_pData = NULL; }
> >
>
> Unrelated to iterators, but the following ctor should probably be
> explicit. Also I wouldn't use the pointer typedef to specify the
> argument type. It works here but conceptually both are unrelated.
>
> >         const_iterator(pointer p)
> >             { m_pData = p; }
> >
>
> I'll comment on this one later.
>
> >         const_iterator(iterator it)
> >             { m_pData = it.m_pData; }
> >
>
> This is unneeded for my solution
>
> >         friend class iterator;
>
> <snip>
>
> You do not need to use iterator_facade for iterator as you can adapt the
> const_iterator.
>
> >     // Read-write iterator:
> >     typedef boost::iterator_facade< iterator,
> >                                     unsigned char,
> >                                     boost::writable_lvalue_iterator_tag,
> >                                     boost::random_access_traversal_tag
> >                                 > boost_iterator;
> >
> >     class iterator
> >         : public boost_iterator
> >     {
> >     public:
> >
> >         iterator()
> >             { m_pData = NULL; }
> >
> >         iterator(pointer p)
> >             { m_pData = p; }
> >
>
> This is wrong. This will give const_iterator -> iterator conversion. You
> certainly don't want this.
>
> >         iterator(const_iterator it)
> >             { m_pData = const_cast<pointer>(it.m_pData); }
> >
>
> <snip>
>
> like this
>
> typedef boost::iterator_adaptor<
>              iterator
>            , const_iterator
>            , unsigned_char
>            , writable_lvalue_iterator_tag,
>            , unsigned char &
>          > iterator_base
>
> class iterator :
>    public iterator_base
> {
>    friend class boost::iterator_core_access;
>
> public:
>    iterator() {}
>
>    explicit iterator(unsigned char* p)
>      : iterator_base(const_iterator(p))
>    {}
>
> private:
>    reference dereference() const
>    {
>      return const_cast<unsigned char&>(*base_reference());
>    }
> };
>
> Given this the const_iterator conv' ctor can be implemented like this
>
> const_iterator::const_iterator(iterator const& it)
>    : m_pData(it.base().m_pData)
> {
> }
>
> HTH
>
> Thomas
>
> _______________________________________________
> Unsubscribe & other changes:
http://lists.boost.org/mailman/listinfo.cgi/boost
>