Re: [Boost-users] iterator_adaptor: making dereference of adapted tree node* return node value

Subject: Re: [Boost-users] iterator_adaptor: making dereference of adapted tree node* return node value
From: Jeffrey Lee Hellrung, Jr. (jeffrey.hellrung_at_[hidden])
Date: 2012-02-24 10:25:06

On Fri, Feb 24, 2012 at 2:51 AM, Rob Desbois <rob.desbois_at_[hidden]> wrote:

> On 23 February 2012 14:39, Robert Jones <robertgbjones_at_[hidden]> wrote:
> > I favoured the type traits magic approach, and publish the resulting
> value
> > type as the
> > value type of the iterator...
> >
> > typedef typename if_<is_const<NODE>, const typename NODE::value_type,
> typename NODE::value_type>::type value_type;
>
> Thanks for the input Rob - see below for the solution I settled on.
>
> On 23 February 2012 18:58, Jeffrey Lee Hellrung, Jr.
> <jeffrey.hellrung_at_[hidden]> wrote:
> > To reiterate Robert Jones' reply, yes, AFAIK, a const iterator should
> pass a
> > const-qualified value_type to iterator_facade/iterator_adaptor, as
> implied
> > by the constant node_iterator example in the docs:
> >
> >
> http://www.boost.org/doc/libs/1_48_0/libs/iterator/doc/iterator_facade.html#a-constant-node-iterator
>
> Actually iterator_facade/iterator_adaptor's value type in the
> tutorials is the _node_ type, not the node's value_type.
>

I never said anything to the contrary, AFAIK...

> On a related note, you should specify your reference type you pass to
> > iterator_adaptor explicitly as the return type of your dereference member
> > function. Otherwise I think you'll end up returning a reference to a
> > temporary within operator*. I believe that's *actually* what the quoted
> > error is trying to tell you.
>
> Thanks - I've updated to use
> node_iterator::iterator_adaptor_::reference as the return type from
> dereference(). The docs show that this defaults to Value&.
> The error message still stands though, since const
> node_type::value_type is still not const.
>

Huh? I don't understand. "const node_type::value_type" *is* const-qualified.

IIRC, in your original post, your dereference member function returned an
rvalue (based on the member function definition, not the actual return
type). This suggests that the reference type you give to iterator_facade
should be Value, rather than accepting the default of Value&.

> I settled on implementing the boost::mpl::if_ method as shown by Rob,
> which turned out to be a viable solution to the problem I faced, but
> then after some more thinking and coding realised that I didn't
> actually want the mutable iterator type to enable modification of the
> data in the referenced node. Since this is an ordered tree, modifying
> the data of a node is clearly a Bad Thing!
> Feeling somewhat silly that I didn't recognise this before leaping
> down the rabbit hole...what I now have is that Value is *always*
> const:
>   template< class NODE >
>   node_iterator: public boost::iterator_adaptor<
>      node_iterator<NODE>, // Derived
>      NODE*, // Base
>      const typename NODE::value_type // Value
>   >
>
> The reference type is then const typename NODE::value_type& as
> expected and everything works fine. It was only after discovering that
> std::set and friends' iterators can be the same type as the
> const_iterator that this finally clicked!
>
> Thanks for the guidance, nice to know I wasn't going completely off my
> rocker with the mpl approach :-D
> --rob
>

Okay, good to know you got things working, I just wanted to give you a
head's up on double-checking to ensure you're not returning an lvalue
reference bound to a temporary :)

- Jeff