Re: [boost] Proposed interface change to boost::algorithm::copy_while

Subject: Re: [boost] Proposed interface change to boost::algorithm::copy_while
From: Marshall Clow (mclow.lists_at_[hidden])
Date: 2013-02-21 11:23:28

On Feb 20, 2013, at 1:05 PM, Neil Groves <neil_at_[hidden]> wrote:

> On Wed, Feb 20, 2013 at 6:55 PM, Marshall Clow <mclow.lists_at_[hidden]>wrote:
> 
>> I'm proposing to change the interface to:
>> 
>>        template<typename InputIterator, typename OutputIterator, typename
>> Predicate>
>>        std::pair<InputIterator, OutputIterator>
>>        copy_while ( InputIterator first, InputIterator last,
>> OutputIterator result, Predicate p );
>> 
>> i.e, changing the return type to return both iterators.
>> 
>> 
> I think that certainly a change needs to be made to accomodate the extra
> information. I wonder how wise it is to return a pair that when the
> InputIterator type is the same as the OutputIterator type that it could be
> mistaken for, and accepted by, range algorithms as input? (That's my fault
> - doh!)

Whoa. That's something I didn't know about. Boost.Range will take std::pair<Iter, Iter> as a range?
Off to do more reading. ;-)

> I had similar agonising moments with the range return types of many
> algorithms and eventually decided that I would provide a template parameter
> that the user could select to choose the return type. The boost::unique
> algorithm is one example of such an affected function. See
> http://www.boost.org/doc/libs/1_53_0/libs/range/doc/html/range/reference/algorithms/introduction.html
> 
> Of course, this might be over-the-top and perhaps it is better to provide
> no such option. This judgement I leave in your capable hands. It is another
> option that could potentially provide backward compatibility.
> 
> 
>> What this means:
>> * If you're not calling copy_while (or copy_until), then this change won't
>> affect you.
>> * If you're not using the return value, then this change won't affect you.
>> * If you are using the return value, then you will have to change your
>> code thus:
>>        Old     foo = copy_while ( first, last, out, p );
>>        New:    foo = copy_while ( first, last, out, p ).second;
>> * if you were not using these calls because they didn't return the input
>> iterator, now you can.
>> 
>> Questions? Comments?  (Except for the "How could you miss that when you
>> wrote those routines?" - I've already asked myself that)
>> Improvements?
>> 
>> 
> My comments are above, but ultimately I think you are on the right track
> and that the change will not be a painful one.

Thanks for the feedback.
I've checked in the proposed change (and tests!) as revision 83064.

More comments welcomed.

-- Marshall

Marshall Clow     Idio Software   <mailto:mclow.lists_at_[hidden]>

A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait).
        -- Yu Suzuki