John Torjo <john.lists@torjo.com> writes:
When doing the formal review, please answer the following questions:
1. What is your evaluation of the design?
It seems to introduce a great deal syntactic noise: std::cout << boost::io::formatob( vec ); // output: [ 1, 2, 3 ] std::cout << boost::io::formatob( vec ).format( "{ ", " }" ); // output: { 1 : 2 : 3 } I'd like to (at least) see rationale on why something cleaner wasn't chosen. For example: namespace io = boost::io; std::cout << io::sequence( vec ); // output: [ 1, 2, 3 ] std::cout << io::sequence( vec, "{ ", " }" ); // output: { 1 : 2 : 3 } In case it's not obvious, I don't like the name "formatob." Abbrevs should generally be avoided, and frankly I don't know what 'ob' stands for. Also "format" doesn't seem to add much semantically. I'm generally not a great fan of statefulness, but this also seems like a reasonable thing to want: namespace io = boost::io; std::cout << io::sequence_delimiters("{ ", " }"); std::cout << io::sequence( vec ); // output: { 1, 2, 3 } Library organization seems problematic. On one hand, I'd like to see the io for individual boost types like octonion distributed across their respective libraries (e.g. in boost/math/io.hpp). On the other hand that will introduce an "apparent library dependency" that may not be needed by some. The "apparent dependency" of this library on all the others isn't very comforting, either. Perhaps some refactoring is appropriate. Another point: I'd like to see functionality for pretty-printing sequences. It's very common to have sequences that are too long to represent comfortably on one line. There are several strategies for dealing with that, and it seems to me that to be *really* indispensible, this library should help in that department. It might also need ways to prevent infinite recursion in self-referential sequences (consider sequences that embed ranges). Where's I/O for tuples?
2. What is your evaluation of the implementation?
Haven't looked.
3. What is your evaluation of the documentation?
What little I saw of the tutorial documentation looked readable and comprehensible. The lack of namespace aliases in the examples don't help the library's case. I'd like to see more rationales. AFAICT the docs lack any sort of formalized reference guide, showing interface summaries, what headers to include, requirements, etc. IMO it's unacceptable without that component. Did I miss something? The use of "implementation defined" in the documentation is incorrect. To be correct, the Boost implementation would have to define whatever it is. The appropriate term is "unspecified."
4. What is your evaluation of the potential usefulness of the library?
Sequence/STL printing: For quickly throwing together "script-like" programs and diagnostic purposes, very high. I'm less sure how useful it will be in hardcore application/library programming. I/O for existing Boost types such as rational<>: IMO the fact that we didn't have I/O for these is sort of embarassing, but at the same time it's unclear to me that not having inserters/extractors was a burden for anyone. I can't recall a single post asking where these things were.
5. Did you try to use the library?
Nope.
6. How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
Somewhere between a glance and a quick reading. I admit that my reading was quite cursory and my comments should be judged accordingly. I might do a more in-depth followup if the concerns I've raised are addressed during the review process.
7. Are you knowledgeable about the problem domain?
I guess a little. It hasn't been uncommon for me to want this sort of capability.
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.
The lack of reference docs is a showstopper for me. Aside from that, I guess I'm not sure yet that this library solves enough/important problems to be worth accepting in its current form. I would want to be very convinced that the design was as slick and beautiful as possible, and I'm not there yet. So I'm provisionally at "No." -- Dave Abrahams Boost Consulting http://www.boost-consulting.com