Re: [boost] [Range] patched: boost/range/iterator.hpp

From: Eric Niebler (eric_at_[hidden])
Date: 2006-05-08 13:47:17

David Abrahams wrote:
> "Eric Niebler" <eric_at_[hidden]> writes:
> 
> 
>>David Abrahams wrote:
>>
>>>Hi Thorsten,
>>>
>>>I took the liberty of checking this patch into the HEAD:
>>>

<snip>

>>>
>>>Please let me know if you have any objections.
>>
>>
>>I object. In the current range interface, the meaning of the range 
>>metafunctions are as follows:
>>
>>range_iterator: always return ::iterator
>>range_const_iterator: always return ::const_iterator
>>range_result_iterator: return either ::iterator or ::const_iterator 
>>depending on the const-ness of the argument.
> 
> 
> Ugh, I forgot about the existence of range_result_iterator.
> 
> 
>>This was deemed an unfortunate design, and there is a reworking of
>>the Range interface in the pipeline. Thorsten wanted to apply it for
>>1.34, but it was too late in the process.
>>
>>Your patch gets everything all mixed up. 
> 
> 
> That's rather a bald and sweeping statement to make without any
> explanation.  What do you really mean by "everything" in this case?

Everything, in this case, means the separate and orthogonal 
responsibilities of the three range metafunctions.

This is a potentially breaking change. Consider a range adaptor...

template<typename Range> struct some_adaptor
{
     typedef typename range_iterator<Range>::type iterator;
     typedef typename range_const_iterator<Range>::type const_iterator;
...
};

In this case, you really don't care about the const-ness of Range -- you 
just want the nested ::iterator type. And anyplace someone is using 
these metafunctions to perform type computations where const-ness 
doesn't matter will now be broken.

>>My suggestion is that you back 
>>this change out and Thorsten applies his changes to HEAD.
> 
> 
> Why?  What other changes do you think are needed in order to make this
> consistent?

Because your change is unnecessary (just use range_result_iterator) and 
because it could potentially break code.

> Anyway, now that you've reminded me of range_result_iterator, I'll be
> happy to roll back my change if this is breaking any code or if
> Thorsten asks me to.

I also object to patching someone else's code without permission unless 
that person is unreachable, which Thorsten is not. Had you checked 
first, he (or I) would have simply told you to use range_result_iterator.

-- 
Eric Niebler
Boost Consulting
www.boost-consulting.com