Formal Review: iostate

From: Beman Dawes (bdawes_at_[hidden])
Date: 2002-02-11 12:06:20

(This review is my own personal opinion, separate from my role as review 
manager.)

Daryle Walker's submission should be accepted by Boost.

Docs
----
The docs should begin with an "Introduction" paragraph which should include 
a "hello, world" level example.
I found myself reading the header to see exactly what classes were 
supplied.  The table isn't horrible, but I'm wondering if others also found 
it a bit unclear as to what exactly is delivered?
Granularity
-----------
The fine granularity of the savers gets in the way for certain 
applications.  For example, I've got some code like this:
     //  function reports job statistics
     void my_show_stats()
     {
       cout << ... ; // no problem
       third_party_show_stats(); // alters state of cout:-(
       cout << ... ; // weird things happen
     }
So I'd like to rewrite it like this:
     //  function reports job statistics
     void my_show_stats()
     {
       cout << ... ; // no problem
       {
         boost::io::ios_all_saver ioasvr( cout );  // save all state info
         third_party_show_stats(); // alters state of cout
       }
       cout << ... ; // still no problem
     }
Unless I'misreading the docs, I can't do that with a single saver because 
the granularity is so fine.
Instead, I'd have to write up to 11 different savers:
     ios_flags_saver
     ios_precision_saver
     ios_width_saver
     ios_iostate_saver
     ios_exception_saver
     ios_tie_saver
     ios_rdbuf_saver
     ios_fill_saver
     ios_locale_saver
     ios_iword_saver
     ios_pword_saver
It isn't that the current fine granularity is wrong; it is exactly what is 
needed for some applications.  But for other applications, additional 
superset savers would seem to be useful.  Have you given this any thought?
Namespace and namespace issues
------------------------------
The submission supplies two headers - <boost/io_fwd.hpp> and 
<boost/io/iostate.hpp>
The contents of the library are placed in namespace boost::io.
For consistency, shouldn't the forward declaration header be named 
<boost/iostate_fwd.hpp>?
For such a simple library with well distinguished names, it seems to me 
that having a separate header sub-directory with only one file in it is 
overkill.  Likewise a separate sub-namespace.  I wish Boost had a agreed 
upon policy (with web page describing it) for these header sub-directory 
and sub-namespace issues.
Testing
-------
I tried the test program under the usual Win32 compilers.  Metrowerks 
passed, but the others all failed. I've sent the log to Daryle. Since other 
people have reported success with various compilers, I'm assuming the 
issues can be resolved without major surgery.
Overall, this is a nice addition to Boost.  Thanks,
  --Beman