[boost] string_algo Formal Review

From: Beman Dawes (bdawes_at_[hidden])
Date: 2003-10-20 08:05:18

Although I certainly think the library fills a very real need for 
additional string algorithms, there are serious problems with the library 
as proposed. Since the problems prevent a complete review, I vote that this 
library not be accepted.

Here are the serious problems that prevent a complete review:

Two libraries intertwined
-------------------------

There really two libraries here:

-- Some additional (and welcome) low level algorithms, particularly useful 
with strings.

-- A framework of Container Traits, Sequence Traits, and iterator_ranges 
which can be used to construct algorithmic functions interfaces and 
implementation at a slightly higher level than traditional begin, end, 
iterator arguments.

Mixing the two together in one review causes me great concern. I'd really 
like to evaluate the framework aspect by seeing it applied to the familiar 
Standard Library STL algorithms. If the framework isn't beneficial for 
those existing algorithms, I'm really not interested in it for strings 
alone. Yet all the framework uses in the library deal with the new string 
algorithms.

Likewise, I'd like to review the string algorithms separately. As it stands 
now, when I try to review one of the string algorithms, I get lost in 
issues relating to the framework and can't give the algorithm itself full 
attention.

Failure to specify foundation algorithms
----------------------------------------

The foundation of any STL-like algorithm extension library are the 
functions which operate on iterators and iterator ranges. If these are 
deficient, convenience wrappers only paper-over the problems.

Yet in the string_algo library, the foundation algorithms are placed in 
namespace detail and are not documented. Thus it isn't possible to 
seriously review them. (Reading the code doesn't count, as far as I'm 
concerned.)

Underspecification
------------------

The documentation is so short on specifics that it is not possible to 
evaluate the library. For example, the "trim" function is specified thus:

    Synopsis

    template<typename ContainerT>
      ContainerT &
      trim(ContainerT & Input, const std::locale & Loc = std::locale());

    Description

    Parameters

    Input
      An input container

    Loc
      A locale used for 'space' classification

    Remove all leading and trailing spaces from the input. The input
    sequence is modified in-place.

    Returns: A reference to the modified input

The lack of any further specification (such as requirements or concepts) 
for ContainerT prevents understanding the limitations of the function. This 
same lack of specification for template parameters is pervasive throughout 
the library. Likewise, the documentation pervasively fails to specify 
preconditions, exceptions which might be thrown, or postconditions. 
Specification of effects or returns, where present, tends to be informal 
and underspecified. These documentation failures prevent a reviewer from 
seriously reviewing the library.

For the iterator_range packaging of two iterators into an object which 
provides a container interface, it is particularly important to know the 
concepts/requirements on the iterators, because these flow through to the 
resulting container. Without any specification or documentation about how 
the category of iterator affects the resulting pseudo container, it is hard 
to know if this is a worthwhile approach or not.

I believe these big-picture problems need to be addressed before the 
libraries can be fully reviewed.

--Beman