
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