[boost] string algo review (part 1)

From: John Maddock (john_at_[hidden])
Date: 2003-10-22 06:54:29

The following is my initial impression of the string library based upon a
review of the documentation, there is an awful lot here, so there is likely
to be more to come (so no vote just yet).

1) The introduction and usage sections look good: some initial thoughts:

a) The iterator_range class appears to offer a subset of the functionality
of boost::half_open_range.  However I couldn't easily locate and
documentation for half_open_range (Dave?), whatever we should combine these
somehow.

b) The split functionality appears to duplicate the functionality provided
by boost::token_iterator and boost::regex_token_iterator - in general
iterators provide a superior (pull) interface to that provided by split -
that's why regex is deprecating regex_split in favour of
regex_token_iterator.

I agree with Beman that some of the support code should be brought forward
for separate review - I'm less worried here about implementation details
like the container traits - than I am about the public interface and the
iterator_range/half_open_range issue.

2) The reference section: we need an overview of the algorithms provided (I
think someone suggested a table which looks to be a good idea), as it stands
the user is faced with a long list of headers and not that much clue what's
actually available.

3) Like others I'm not entirely sure that making the inplace versions of
these algorithms the default is a good idea, personally I would prefer to
see a name suffix, like to_upper_inplace or something (but lots of good
suggestions have already been made on this issue).

4) case_conv.hpp: Looks Ok at first sight.

5) classification.hpp: The return type of these functions is an undocumented
detail type (string_algo::detail::is_classifiedF), which should probably be
a documented functor type IMO (compare with std::binder1st and bind1st for
example).  It also wasn't clear to me what these functions did at first
sight from the introductory blurb, and now that it is clear, I'm not sure
why these functions are provided rather than a raw function object (compare
with std::equal_to etc).

6) container_traits.hpp: needs a separate review: for example why both
container_traits and container_value_type etc.  Why no container_ prefix to
size, empty, begin etc.

More to follow...

John.