Re: [boost] [iterator] [property_map] Enhancement proposals

Subject: Re: [boost] [iterator] [property_map] Enhancement proposals
From: Louis Dionne (louis.dionne92_at_[hidden])
Date: 2012-10-08 12:11:49

Jeffrey Lee Hellrung, Jr. <jeffrey.hellrung <at> gmail.com> writes:

> Regarding accessor_iterator: The implementation is very short, but it still
> seems to have minimal if any advantage to just using a transform_iterator
> directly composed with a function object wrapping the pointer-to-member
> (either at runtime or compile-time [1]). Also, seems you're lacking a
> convenient make_accessor_iterator (which, of course, would probably have to
> be a macro, as described in [1]).

Regarding the make_accessor_iterator:
I did not provide such a function because I did not know how to implement it.
After reading the link, I indeed see how it could be done. I will do it if the
class is accepted.

Another possibility is to set the pointer to member at runtime, which then
makes the implementation of make_accessor_iterator trivial. You can see this
alternative implementation on the dynamic_accessor_iterator branch of the git
repository.

Regarding the usefulness of the class vs using transform iterator:
I too believe the same thing can be achieved using a transform_iterator. It
really depends on whether you think creating a function object is a pain. It
would be useful to know what others think of it to know whether there is a
need in the community.

> This could be useful, but there's a few things I need to understand about
> your implementation first.
> - Using is_convertible to determine iterator convertibility is, strictly
> speaking, too loose, as it allows derived* -> base* conversions that are
> undesirable if sizeof( derived ) > sizeof( base ). At some point I probably
> need to fish through the Iterator library and change any such uses myself...

Given the 3 places where I use it currently:

    template <typename OutIter>
    explicit chained_output_iterator(Function const&, OutIter const&,
        typename enable_if_convertible<OutIter, OutputIterator>::type* =0);

    template <typename OutIter>
    chained_output_iterator(OutIter const&,
        typename enable_if_convertible<OutIter, OutputIterator>::type* =0);

    template <typename Func, typename OutIter>
    chained_output_iterator(chained_output_iterator<Func, OutIter> const&,
        typename enable_if_convertible<Func, Function>::type* =0,
        typename enable_if_convertible<OutIter, OutputIterator>::type* =0);

I take it that you are referring to the first two copy constructors being too
loose on the `OutIter' type. Would the following be alright, given that
OutputIterator is the type of the wrapped iterator?

    explicit chained_output_iterator(Function const&, OutputIterator const&);
    chained_output_iterator(OutputIterator const&);

> - I'm wondering why the increment of the wrapped iterator happens inside
> proxy::operator= rather than chained_output_iterator::operator++...? Also,

You are right, this is my mistake and it's fixed on master now.

Since this change would make the implementation with function_output_iterator
more complicated than the `from scratch' implementation, I believe
function_output_iterator should not be used.

Note: The implementation using function_output_iterator comes from a
      suggestion in your first reply and is available on the
      using_function_output branch of the repository.

> typically operator* is a const member function, and moving the advancement
> of the wrapped iterator into chained_output_iterator::operator++ would
> allow that. I might be missing something though.

In order to make the chained_output_iterator::operator* const, we would need
to make the proxy hold copies or const references to the wrapped iterator and
function. However, using const references would add the following
restrictions:
    - the wrapped iterator's operator* must be const
    - the function object's operator() must be const

For these reasons, I believe the proxy should either store copies or we should
give up making chained_output_iterator::operator* const. I'll wait for your
reply before I change that.

Louis Dionne