[boost] Re: Formal review of "Output Formatters" library begins today

From: Thorsten Ottosen (nesotto_at_[hidden])
Date: 2004-09-12 07:46:02

Hi Reece,

Here are some general comments:
----------------------------------

0. I think the library should be put in the directory
"boost/output_format"
I hate cryptic names.

1. In this example

   std::cout << boost::io::formatob( vec );
   // output: [ 1, 2, 3 ]
   std::cout << boost::io::formatob( vec ).format( "{ ", " }" );
   // output: { 1 : 2 : 3 }

why does the change of braces change "," to ":" ?

2. In this example

      int a[] = { 5, 4, 3, 2, 1 };
      std::cout << boost::io::formatob( boost::io::range( a, a + 5 ));
      std::cin  >> boost::io::formatob( boost::io::range( a, a + 5 ));
      // output: [ 5, 4, 3, 2, 1 ]

it might be good to support a range version with the posibility to

1. throw if there is too few elements
2. default initialize if there is too few elements

3. cryptic name: formatob.hpp && formatobex; should be format_object and
whatever the latter really means

4. given std::cout << boost::io::formatobex< std::string >( v );

   maybe std::string should be a default argument?

5. A lot of boost::io::range() can be replaced with one Taking a Single Pass
Range

6. STL IO: It is not obvious how insertion is done? With push_back or by
overwriting elements?

7. why is it necessary with
           boost::io::formatob( boost::io::range( a, a + 5 ));
   could we not just say
           boost::io::range( a, a + 5 );

8. It should be possible to say

    vector<int> v = ...;
    cout << v;

without the format_ob() if I include stl.hpp, ie, overloads for all standard
types.

9. I would prefer basicfmt_t to be basic_format_type or perhaps basic_fmt_type
if the "fmt" shorthand is used consistently etc. That is, all the cryptic
    concatenation should be removed.

And now my review comments.

| When doing the formal review, please answer the following questions:
|
| 1. What is your evaluation of the design?

seems ok. I'm a little irritated about too cryptic names, concatenation and _t
types.

| 2. What is your evaluation of the implementation?

haven't looked.

| 3. What is your evaluation of the documentation?

good, although I miss return-types and template paramaters and some synopsises
with better overview.
For example, what does boost::io::formatobex< DelimeterType >( const T & ob );
return? Maybe it doesn't matter, but
then it should say <i>Implementation-defined</i>boost::io::formatobex<
DelimeterType >( const T & ob ); etc.

| 4. What is your evaluation of the potential usefulness of the library?

quite useful.

| 5. Did you try to use the library?
|     With what compiler?  Did you have any problems?

no.

| 6. How much effort did you put into your evaluation?
|     A glance? A quick reading? In-depth study?

a quick glance.

| 7. Are you knowledgeable about the problem domain?

no.

| And most important,
| 8. Do you think the library should be accepted as a Boost library?
| Be sure to say this explicitly so that your other comments don't obscure
| your overall opinion.

yes. I think all issues can be dealt with quite easy for a post-review.

br

Thorsten