RE: [boost] Re: Formal review of "Output Formatters" library beginstoday

From: Reece Dunn (msclrhd_at_[hidden])
Date: 2004-09-12 12:39:41

David Abrahams wrote:
>John Torjo <john.lists_at_[hidden]> 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