RE: [boost] Re: Formal review of "Output Formatters" library beginstoday
David Abrahams wrote:
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 }
There are two basic components: a delimeter holder (formatter/openclose_formatter) that contain the delimeters and control over them, and a format object that tells the library what type the object being formatted is. Your suggestion for placing the delimeter specifications in the type construction messes things up since you are combining two separate concepts. This would lead to many different function overloads (you'd need one for each variant). Also, the design would fail when adding new delimeters for things like trees and graphs.
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.
"ob" stands for "object". I am willing to negotiate appropriate names. io::sequence seems reasonable enough.
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 }
You can achieve something like this using: io::formatter< const char * > fmt( "{ ", " }" ); std::cout << io::sequence( vec ).format( fmt ); If there are any better alternatives, I am willing to listen.
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.
I think this will be an issue no matter how it is implemented. Especially for the STL container types. The code has already been refactored so that each STL/Boost type registers its self in a separate file, so you can include them individually. It might be a good idea to have the Boost/complex types include just the type deduction mechanism code. This would reduce the file/library dependencies.
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.
I don't know if additional support needs to be built into the framework, but you can implement your own format object that could perform pretty-printing functionality, e.g.: std::cout << io::sequence( vec, io::pretty_container()); It should also be possible to design character escapers/unescapers, allowing: std::cout << io::sequence( str, io::container_fmt( xml::char_escaper().format( "", "" ));
It might also need ways to prevent infinite recursion in self-referential sequences (consider sequences that embed ranges).
Hmm. I am not sure how to implement this. Suggestions are welcome.
Where's I/O for tuples?
I have not added tuple support yet.
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.
okay.
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?
okay. IIUC, Doxygen will auto-generate these when provided javadoc-like comments.
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."
okay.
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.
I/O does exist for rational, octonion, interval and so forth. The headers in my library for boost components are there to tell my library what type of sequence a particular type is (e.g. rational<> is a pair type, octonion an n-ary type and std::list a container). This is so that my library knows how to format that type.
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.
okay. I'll look at getting doxygen up and running on my machine. Regards, Reece _________________________________________________________________ It's fast, it's easy and it's free. Get MSN Messenger today! http://www.msn.co.uk/messenger
"Reece Dunn" <msclrhd@hotmail.com> writes:
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 }
You can achieve something like this using:
io::formatter< const char * > fmt( "{ ", " }" ); std::cout << io::sequence( vec ).format( fmt );
If there are any better alternatives, I am willing to listen.
Well, I think my alternative is better if you want a stateful change, since yours doesn't accomplish that. My suggestion is certainly consistent with normal io manipulators. The point is to stream a bunch of sequences without having to repeat the format part. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
David Abrahams wrote:
"Reece Dunn" <msclrhd@hotmail.com> writes:
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 }
You can achieve something like this using:
io::formatter< const char * > fmt( "{ ", " }" ); std::cout << io::sequence( vec ).format( fmt );
If there are any better alternatives, I am willing to listen.
Well, I think my alternative is better if you want a stateful change, since yours doesn't accomplish that. My suggestion is certainly consistent with normal io manipulators. The point is to stream a bunch of sequences without having to repeat the format part.
Yes, that would be very neat. And it's certainly doable: ios_base::pword Best, John -- John Torjo -- john@torjo.com Contributing editor, C/C++ Users Journal -- "Win32 GUI Generics" -- generics & GUI do mix, after all -- http://www.torjo.com/win32gui/ -- v1.4.0 - save_dlg - true binding of your data to UI controls! + easily add validation rules (win32gui/examples/smart_dlg)
"Reece Dunn" <msclrhd@hotmail.com> writes:
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?
okay. IIUC, Doxygen will auto-generate these when provided javadoc-like comments.
You won't get Concept documentation from Doxygen unless you take care to actually write it ;-), and regardless I don't think this can be done as an afterthought. I would be wary of taking this route unless your implementation is very simple. It's important to present a coherent view of the *user interface* that doesn't expose implementation details. For example, sometimes public inheritance from some helper class is neccessary as an implementation technique, and you don't want to expose it, but you need to expose the public members of that helper. I don't think Doxygen can deal with that.
The lack of reference docs is a showstopper for me.
okay. I'll look at getting doxygen up and running on my machine.
It'll be interesting to see how it works out. Maybe I'm being overly skeptical. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
On Sun, 2004-09-12 at 14:26, David Abrahams wrote:
"Reece Dunn" <msclrhd@hotmail.com> writes:
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?
okay. IIUC, Doxygen will auto-generate these when provided javadoc-like comments.
You won't get Concept documentation from Doxygen unless you take care to actually write it ;-), and regardless I don't think this can be done as an afterthought. I would be wary of taking this route unless your implementation is very simple. It's important to present a coherent view of the *user interface* that doesn't expose implementation details. For example, sometimes public inheritance from some helper class is neccessary as an implementation technique, and you don't want to expose it, but you need to expose the public members of that helper. I don't think Doxygen can deal with that.
The Gtkmm project deals with this by adding the macro DOXYGEN_SHOULD_SKIP_THIS to the PREDEFINED field in the Doxyfile. Parts of the code that should be not be in the user-level docs then get bracketed with #if DOXYGEN_SHOULD_SKIP_THIS / #endif. The GNU libstdc++ library uses a variation of this technique to generate separate user-level and maintainer-level documentation. HTH, -Jonathan Brandmeyer
Jonathan Brandmeyer <jbrandmeyer@earthlink.net> writes:
You won't get Concept documentation from Doxygen unless you take care to actually write it ;-), and regardless I don't think this can be done as an afterthought. I would be wary of taking this route unless your implementation is very simple. It's important to present a coherent view of the *user interface* that doesn't expose implementation details. For example, sometimes public inheritance from some helper class is neccessary as an implementation technique, and you don't want to expose it, but you need to expose the public members of that helper. I don't think Doxygen can deal with that.
The Gtkmm project deals with this by adding the macro DOXYGEN_SHOULD_SKIP_THIS to the PREDEFINED field in the Doxyfile. Parts of the code that should be not be in the user-level docs then get bracketed with #if DOXYGEN_SHOULD_SKIP_THIS / #endif. The GNU libstdc++ library uses a variation of this technique to generate separate user-level and maintainer-level documentation.
I am talking about situations where the inherited members should really be documented as members of the _derived_ class. That was very common for me in Boost.Python, and I don't think you can do anything as trivial as marking regions to be skipped if you want to achieve it. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
On Sun, 2004-09-12 at 21:45, David Abrahams wrote:
Jonathan Brandmeyer <jbrandmeyer@earthlink.net> writes:
You won't get Concept documentation from Doxygen unless you take care to actually write it ;-), and regardless I don't think this can be done as an afterthought. I would be wary of taking this route unless your implementation is very simple. It's important to present a coherent view of the *user interface* that doesn't expose implementation details. For example, sometimes public inheritance from some helper class is neccessary as an implementation technique, and you don't want to expose it, but you need to expose the public members of that helper. I don't think Doxygen can deal with that.
The Gtkmm project deals with this by adding the macro DOXYGEN_SHOULD_SKIP_THIS to the PREDEFINED field in the Doxyfile. Parts of the code that should be not be in the user-level docs then get bracketed with #if DOXYGEN_SHOULD_SKIP_THIS / #endif. The GNU libstdc++ library uses a variation of this technique to generate separate user-level and maintainer-level documentation.
I am talking about situations where the inherited members should really be documented as members of the _derived_ class. That was very common for me in Boost.Python, and I don't think you can do anything as trivial as marking regions to be skipped if you want to achieve it.
Well, you can reverse the process to have Doxygen see the declarations of functions that are implemented in the parent: #ifndef DOXYGEN_SHOULD_SKIP_THIS namespace detail { class base { public: void some_func(); }; } #endif #ifndef DOXYGEN_SHOULD_SKIP_THIS class public : public base # else class public #endif { // ... #ifdef ONLY_DOXYGEN_SHOULD_SEE_THIS /** This function does stuff */ void some_func(); #endif }; However, the result is somewhat unclean, and since the docs are no longer at the site of the code they are actually documenting, you've lost the principle benefit of using Doxygen in the first place. That said, I think it would be very difficult to implement an in-code documentation system that handled this case cleanly. -Jonathan
Jonathan Brandmeyer <jbrandmeyer@earthlink.net> writes:
On Sun, 2004-09-12 at 21:45, David Abrahams wrote:
Jonathan Brandmeyer <jbrandmeyer@earthlink.net> writes:
You won't get Concept documentation from Doxygen unless you take care to actually write it ;-), and regardless I don't think this can be done as an afterthought. I would be wary of taking this route unless your implementation is very simple. It's important to present a coherent view of the *user interface* that doesn't expose implementation details. For example, sometimes public inheritance from some helper class is neccessary as an implementation technique, and you don't want to expose it, but you need to expose the public members of that helper. I don't think Doxygen can deal with that.
The Gtkmm project deals with this by adding the macro DOXYGEN_SHOULD_SKIP_THIS to the PREDEFINED field in the Doxyfile. Parts of the code that should be not be in the user-level docs then get bracketed with #if DOXYGEN_SHOULD_SKIP_THIS / #endif. The GNU libstdc++ library uses a variation of this technique to generate separate user-level and maintainer-level documentation.
I am talking about situations where the inherited members should really be documented as members of the _derived_ class. That was very common for me in Boost.Python, and I don't think you can do anything as trivial as marking regions to be skipped if you want to achieve it.
Well, you can reverse the process to have Doxygen see the declarations of functions that are implemented in the parent:
#ifndef DOXYGEN_SHOULD_SKIP_THIS namespace detail { class base { public: void some_func(); }; } #endif
#ifndef DOXYGEN_SHOULD_SKIP_THIS class public : public base # else class public #endif { // ... #ifdef ONLY_DOXYGEN_SHOULD_SEE_THIS /** This function does stuff */ void some_func(); #endif };
However, the result is somewhat unclean, and since the docs are no longer at the site of the code they are actually documenting, you've lost the principle benefit of using Doxygen in the first place. That said, I think it would be very difficult to implement an in-code documentation system that handled this case cleanly.
Of course I disagree ;-) You just need a collapse-to-derived directive with which you can annotate a class. I wonder how Synopsis is doing these days... -- Dave Abrahams Boost Consulting http://www.boost-consulting.com
participants (4)
-
David Abrahams -
John Torjo -
Jonathan Brandmeyer -
Reece Dunn