Re: [boost] Re: Advice on using iterator_adaptor

From: Thomas Witt (witt_at_[hidden])
Date: 2003-09-30 13:41:52

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