[boost] IOStreams formal review

From: John Torjo (john.lists_at_[hidden])
Date: 2004-09-06 07:18:23

-------------------------------------------
     *  What is your evaluation of the design?

Very nice and clean.

-------------------------------------------

     * What is your evaluation of the implementation?

-------------------------------------------
     * What is your evaluation of the documentation?

Documentation is very nice and neat.

Small comments:

tutorial.html:

" The Iostreams Library allows users to create a standard input stream 
or stream buffer by defining a classes with a single member function 
read and several member types."
Q: What do you mean, by 'several member types'?

"Buffering and the ability to put back characters come for free."
Q: Can I putback multiple chars, or just one?

3.4 Output Filters

Here your examples use 'filtered_streambuf', while in the Reference 
section, you use 'filtering_streambuf'. Is this a typo?

examples.html

I love the presidential filter ;)

policy_based_streams.html:

I think you should visually show that streambuf_facade owns the policy 
(the source). I had to look through the code to realize that. I have 
later realized that you did say at the beginning " by delegating to a 
contained instance of a policy class". Maybe emphasize the "contained 
instance" (bold or something)?

filtering_streambuf.html:
Again, by looking at the code, it seems that you own the streambufs. Is 
that so? If it is, please state it in the docs (this is more of a 
general issue - it applies to other classes as well).
I am worried about this, since I want to know what I need to destroy 
manually, and what is destroyed automatically for me.

General issues:
I'm not sure if mixing streambuf and stream concepts is such a good idea.
As I've seen in your examples, you only use streams. Tnus, you don't use 
streambufs directly.

While having streambuf_facade is DEFINITELY a good idea, I think you 
should have an  "Advanced" section, where you talk about the streambufs. 
The rest of us are quite happy with having and using stream_facade.

-------------------------------------------

     * What is your evaluation of the potential usefulness of the library?

Very useful

-------------------------------------------

     * Did you try to use the library?  With what compiler?  Did you 
have any problems?

VC7.1 Compiled some examples - they worked ok, after I slightly modified 
the jamfile.
(which makes me not like bjam even more - it makes it so hard to build 
even trivial things)

-------------------------------------------

     * How much effort did you put into your evaluation? A glance? A 
quick reading? In-depth study?

Reviewed it several times (in the past as well).

-------------------------------------------

     * Are you knowledgeable about the problem domain?
Yes

-------------------------------------------

     * 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.

I think this library should be ACCEPTED.

Best,
John

-- 
John Torjo
Freelancer
-- john_at_[hidden]
Contributing editor, C/C++ Users Journal
-- "Win32 GUI Generics" -- generics & GUI do mix, after all
-- http://www.torjo.com/win32gui/
-- v1.3beta released - check out splitter/simple_viewer,
    a File Explorer/Viewer all in about 200 lines of code!
Professional Logging Solution for FREE
-- http://www.torjo.com/code/logging.zip (logging           - C++)
-- http://www.torjo.com/logview/         (viewing/filtering - Win32)
-- http://www.torjo.com/logbreak/        (debugging         - Win32)

# Boost.Iostreams Library Build Jamfile

# (C) Copyright Jonathan Turkanis 2004
# Distributed under the Boost Software License, Version 1.0. (See accompanying 
# file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt.)

subproject libs/io/build ;

# bring in compression-options template
import ../build/compression ;

local ZLIB = [ cond $(NO_ZLIB) $(NO_COMPRESSION) : : zlib ] ;
local LIBBZ2 = [ cond $(NO_LIBBZ2) $(NO_COMPRESSION) : : bzip2 ] ;

SOURCES = file_descriptor memmap $(ZLIB) $(LIBBZ2) ; 

lib boost_io
    : ../src/$(SOURCES).cpp 
      [ unless $(NO_COMPRESSION) : <template>compression-options ]
    : # build requirements
      [ common-names ] # magic for install and auto-link features
      <include>$(BOOST_ROOT) <sysinclude>$(BOOST_ROOT)
    : debug release  # build variants
      <debug><runtime-link>static
      <release><runtime-link>static
    ;

dll boost_io
    : ../src/$(SOURCES).cpp 
      [ unless $(NO_COMPRESSION) : <template>compression-options ]
    : # build requirements
      [ common-names ]  # magic for install and auto-link features
      <define>BOOST_IO_DYN_LINK=1  # tell source we're building dll's
      <runtime-link>dynamic  # build only for dynamic runtimes
      <include>$(BOOST_ROOT) <sysinclude>$(BOOST_ROOT)
    : debug release  # build variants
    ;

install io lib
     : <lib>boost_io <dll>boost_io
     ;

stage stage/lib : <lib>boost_io <dll>boost_io
    :
      # copy to a path rooted at BOOST_ROOT:
      <locate>$(BOOST_ROOT)
      # make sure the names of the libraries are correctly named:
      [ common-names ]
      # add this target to the "stage" and "all" psuedo-targets:
      <target>stage
      <target>all
    : debug release # build variants
    ;

# end