Re: [boost] string_algo Formal Review

From: Jeff Garland (jeff_at_[hidden])
Date: 2003-10-22 01:12:34

I'll start my review with a small program I wrote today:

#include "boost/string_algo/case_conv.hpp"
#include "boost/string_algo/trim.hpp"
#include "boost/string_algo/replace.hpp"
#include "boost/string_algo/predicate.hpp"
#include <iostream>
#include <string>

int
main()
{
  using namespace boost;
  std::string emphasis("strongly");
  to_upper(emphasis);
  if (iends_with(emphasis, "Ly")) {
    std::string s("  I vote %EM% for acceptance   ");
    to_lower(s);
    replace_all(s, "%em%", emphasis);
    replace_first(s, "i", "I");
    trim_left(s);
    std::cout << "'" << s << "'" << std::endl;
  }
}

output is:
'I vote STRONGLY for acceptance   '

I want to see the string_algo library in boost because string processing is
essential to so many programs.  Unfortunately string processing is still way
too difficult given the tools of standard C++.  This library makes string
processing a joy.  Easy, simple, and obvious.  From what I see, the library
makes the simple tasks simple, and provides provisions for the complex as needed.

To review the library I read the docs and wrote several test programs using
gcc 3.2 on Linux. I also skimmed through several of the implementation headers
and looked through the examples and tests. I also tried to compile,
unsuccessfully, the examples and tests.  However, I believe these compilation
issues are more related to the current state of my boost tree than the library
(mostly regex stuff and regex isn't compiling for me at this moment).  I
believe this to be my own issue and not a problem with the library.  Also, it
allowed me to test directly that the Regex library is only needed if Regex
features are used.  Overall I found the library easy to use and the
documentation and examples to be quite good.  The overall design of the
library seems sound and the source is quite readable.  The test suite appears
to be extensive and should facilitate porting to additional platforms.  

Let me weigh in on a couple of topics that have been discussed:
1) The inplace suffix.  To me adding _inplace to the code above doesn't add
much clarity.  The library convention is clearly stated that if you don't want
the mutating version use an _copy variant. I'm fine with making void returns
for the mutating functions because I'm not big on chaining things together.
2) I'm hopeful that we can conditionally accept the library with modifications
because I'd hate to see this very useful library delayed because the interface
isn't 'enough like STL' (I realize this is a gross oversimplification of
Beman's issue, but I can't think of another way to sum it up).  I believe the
library correctly presents a reasonable interface that makes string
programming from the user perspective easy, while retaining the flexibility of
STL interfaces for the advanced user.

Now for a few new issues:
1) Any reason why we can't put all functions in namespace boost::string_algo
(or boost::algorithm::string)?  I found it a little tricky to figure out which
things were boost versus string_algo.
2) On a related front, I don't really care for the find.hpp, find2.hpp,
replace.hpp, replace2.hpp dichotomy.  As a user I think of using find. There
are several variations, some more complex than others.  But 'find2' doesn't
give me any clues as to when I need to include find2.hpp version find.hpp.
Perhaps find_generic.hpp instead?  Or maybe everything in find.hpp? (Note that
in my reading of the source it appears that all of the zzzz2.hpp headers are
always included in the zzzz.hpp header anyway).
3) There are a number of grammer issues in the docs.  Too many to list here,
but I'll try to help you with some of these offline.  I realize English can be
a challange -- for both native and non-native speakers...

And finally, sincere thanks for all your hard work on producing this library!

Jeff