Re: [boost] Re: string_algo Formal Review

From: Peter Dimov (pdimov_at_[hidden])
Date: 2003-10-23 11:35:04

Pavol Droba wrote:
> On Wed, Oct 22, 2003 at 08:41:42PM +0300, Peter Dimov wrote:
>> Pavol Droba wrote:
>>>
>>> all() predicate was added to the string_algo library, because, there
>>> was a request to handle classification of a string. (i.e a sequence
>>> variant of std::isspace and etc). all() with a set of classification
>>> predicated was a natural choice for this task.
>>
>> I haven't studied the library in enough detail to write a proper
>> review, but
>> that's one of the issues I see with it, there is too much algorithm
>> duplication:
>>
>>     all(x, f) :- find_if(x.begin(), x.end(), not1(f)) == x.end()
>
> Just to mention, that std::not1() requires the predicate to be
> unary_function. It
> is very strong requirement and disallows the use an ordinary function
> as a predicate (not sure about lambda).

This is correct, and one way to address this, assuming that it is an issue,
is:

    It find_if_not(It first, It last, Pred pr);

> Anyway, I don't realy get why are you talking about some imaginary line.

You start with the actual request, assuming that it is legitimate (not all
requests are):

>>> all() predicate was added to the string_algo library, because, there
>>> was a request to handle classification of a string. (i.e a sequence
>>> variant of std::isspace and etc).

This request, in its simplest form, translates to

    bool isspace(std::string const & s);

This is what users want.

You can decide to make it more generic if the increased power is worth the
price, adding a std::locale parameter to match std::isspace, templatizing on
the character type, generalizing to is_classified, generalizing to arbitrary
sequences and arbitrary predicates, and finally generalizing to a range
(pair of iterators).

You can "draw the line" anywhere in between. You have decided to provide
'all' but stopped short of 'find_if_not'.

[...]

> Last point is, why the string_algo supports more different containers.
> String processing is not only a matter of std::string. There are
> other essential structures, for instance C-string.
> It would be possible to constraint the library to std::string, but
> what benefit it would bring over the current design?
>
> Imagine a function:
>
> replace_first( Input, Substring, Replacement );
>
> Under the current design, you can use any container for each of
> parameter. It can be even char*, or char[]. It can be a unicode a
> vector or anything else satisfying the requirements.
>
> You are suggesting this
>
> replace_first( const std::basic_string<>& Input, std::basic_string<>&
> Substring, std::basic_string<>& Replacement );
>
> Eigher you povide additional oveloads for at least char* and wchar*,
> it this interace is very limited.

The interesting thing is that I would not object to your strawman generic

    void replace_first( String, Substring, Replacement );

that works on "anything" since

1. A generic replace_first does everything the non-generic version would do,
and more, and I, as the user, need not pay anything for the increased
functionality;

2. It is fairly natural for replace_first() to be a mutating operation;

and

3. There is no general algorithm disguised as a string processing function
lurking underneath replace_first.

This only goes to highlights my concerns that "algorithm bundles" aren't
well suited for review as one monolithic "take it or leave it" piece.