IOStreams formal review
------------------------------------------- * 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@torjo.com 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
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.
I get the impression that most of the reviewers have an objection against the uses of streams. The consensus seems to be that the library should restrict itself to a (binary) streambuf interface. Almost everyone has complained one way or the other about the mixup of streams versus streambufs. -- Carlo Wood <carlo@alinoe.com>
Carlo Wood wrote:
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.
I get the impression that most of the reviewers have an objection against the uses of streams. The consensus seems to be that the library should restrict itself to a (binary)
Why binary? I could see lots of uses for text as well. My favorite is: write a text that is to be formatted by columns. Every time you write a tab, you go to next column. At the sink' construction, specify how many spaces each column has, and when you receive a tab, fill the rest of the column with spaces.
streambuf interface. Almost everyone has complained one way or the other about the mixup of streams versus streambufs.
Jonathan already answered it. Best, John -- John Torjo Freelancer -- john@torjo.com 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)
On Tue, Sep 07, 2004 at 07:41:25AM +0200, John Torjo wrote:
Why binary? I could see lots of uses for text as well.
Because text is a special case of binary. If the library supports streambufs, and when it would support the concept of 'message blocks', then "text" just means that you'd detect End-Of-Line sequences and cut the stream into messages that are in fact lines of text. Actually putting that text in a std::string and doing some work on it should be in user code imho. -- Carlo Wood <carlo@alinoe.com>
"John Torjo" <john.lists@torjo.com> wrote in message: Thanks for the review!
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'?
Originally it was a char type a traits type and an i/o category tag. Now it's just the char type and category. I was hoping to avoid listing the member types in the tutorial, because by deriving from one of the convenience base classes source, sink, etc., - unless you're writing a filter or resource templated on the char_type, you can avoid dealing with char_type - unless you need optional behavior beyond Closable and Localizable, you don't need to worry about the i/o category tag. Would you suggest a short description of these types in the tutorial, or just a comment that beginners don't need to worry about them, plus a link to a detailed explanation?
"Buffering and the ability to put back characters come for free." Q: Can I putback multiple chars, or just one?
The size of the putback buffer can be specified when the stream or stream buffer is opened using open() or a constructor which specifies a resource -- That's if you're using stream_facade or streambuf_facade. If you're writing a filter, say: struct my_filter : input_filter { template<typename Source> int get(Source& src) { ... how many chars can I putback to src??? } }; Here, it's only safe to put back one character. Currently source is a streambuf_facade with a putback buffer size of 4 (configurable by a macro). However, according to one of the optimizations I am considering, src could sometimes be an arbitrary stream or streambuffer which a user has added to the end of the filter chain.
3.4 Output Filters
Here your examples use 'filtered_streambuf', while in the Reference section, you use 'filtering_streambuf'. Is this a typo?
Yes. Thanks. The current name is 'filtering_streambuf'; in the past I've used 'filtered_streambuf' and 'filter_streambuf'. What's your preference?
examples.html
I love the presidential filter ;)
I was hoping someone would notice that one ;-)
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)?
Lifetime Management of Filters and Resources. This material should either go under 'Concepts' in the User's Guide, or have its own section. A. By default, filters and resources are stored internally by value and must be copy constructible. The reason for pass-by-value (really by const ref) is exception safety. This was accidentally omitted from the most recent rewrite of the rationale. B. It is unspecified whether filters and resources which are copy constructible have deep copy semantics. C. Standard streams and stream buffers are models of Resource; they are always stored by reference. D. The library may make an arbitrary number of copies (usually just one) of a filter or resource, but only one is stored, and no copies are made after i/o begins E. Filters and resources can be stored by reference using the function boost::ref (see http://tinyurl.com/4padg). This is useful in two types of cases: 1. The filter or resource type is not copy-constructible 2. Cases like the one you present below, in which you keep an external instance of a filter, and want changes to this external instance to be reflected directly in the filtered i/o. F. Filters and resources must free all associated resources (in the usual sense) either: 1. When the stored copy is destroyed, or 2. If the filter or resource type models Closable (http://tinyurl.com/3pg5j) and i/o has commenced, when the function boost::io::close() is called.
I believe these principles are fairly intuitive and easy to work with, but
Larry Evans's comments convinced me that I need a prominent section on lifetime management issues. Here's what I suggested: "Jonathan Turkanis" <technews@kangaroologic.com> wrote: they
need to be spelled out in detail somewhere, and probably addressed in the examples.
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.
All the internal streambufs are freed automatically by filtering_streambuf. If you write: filtering_ostream out; out.push(base64_encoder()); // user-defined filter out.push(my_stream); You're responsible for freeing my_stream, if it was dynamically allocated. The library went through a large number of conventions for passing filters and resource (by pointer, by value,...). I forgot to discuss this in the latest rewrite of the rationale. The main point was exception safety.
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.
I think I need to add -- very early in the documentation -- a brief overview of the standard iostreams library, in which I 1. indroduce the types streamsize and streamoff 2. mention that basic_streambuf is the component which represent a connection to a data source/sink 3. explain that streams represent the formatting layer of standard i/o, and that each stream contains a pointer to a streambuf to which its i/o functions and operators delegate. I can then explain that streambuf_facade is the principle component provided by the library, that stream_facade and filtering streams are really only convenience wrappers, but that the tutorial and examples will mostly use the wrappers since these are more familiar to existing users of the standard library and will probably be used most in practice. How does that sound?
-------------------------------------------
* 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)
I'm no expert on Boost.Build -- I got the Jamfile to work on my system by trial and error. Could you explain why you added the lines <debug><runtime-link>static <release><runtime-link>static ?
I think this library should be ACCEPTED.
Great! Thanks.
Best, John
Jonathan
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'?
Would you suggest a short description of these types in the tutorial, or just a comment that beginners don't need to worry about them, plus a link to a detailed explanation?
A link to a detailed info I think would suffice.
"Buffering and the ability to put back characters come for free." Q: Can I putback multiple chars, or just one?
The size of the putback buffer can be specified when the stream or stream buffer is opened using open() or a constructor which specifies a resource -- That's if you're using stream_facade or streambuf_facade.
That's cool!
If you're writing a filter, say:
struct my_filter : input_filter {
template<typename Source>
int get(Source& src)
{
... how many chars can I putback to src???
}
};
Here, it's only safe to put back one character. Currently source is a
For filters I guess I'm ok with a putback of 1.
3.4 Output Filters
Here your examples use 'filtered_streambuf', while in the Reference section, you use 'filtering_streambuf'. Is this a typo?
Yes. Thanks. The current name is 'filtering_streambuf'; in the past I've used 'filtered_streambuf' and 'filter_streambuf'. What's your preference?
filtering_streambuf is better
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)?
Larry Evans's comments convinced me that I need a prominent section on lifetime management issues. Here's what I suggested:
"Jonathan Turkanis" <technews@kangaroologic.com> wrote:
Lifetime Management of Filters and Resources. This material should either go under 'Concepts' in the User's Guide, or have its own section.
[...]
I believe these principles are fairly intuitive and easy to work with, but
they
need to be spelled out in detail somewhere, and probably addressed in the examples.
Yup, this section would be great!
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.
All the internal streambufs are freed automatically by filtering_streambuf. If you write:
filtering_ostream out; out.push(base64_encoder()); // user-defined filter out.push(my_stream);
You're responsible for freeing my_stream, if it was dynamically allocated.
Yes, that makes sense.
The library went through a large number of conventions for passing filters and resource (by pointer, by value,...). I forgot to discuss this in the latest rewrite of the rationale. The main point was exception safety.
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.
I think I need to add -- very early in the documentation -- a brief overview of the standard iostreams library, in which I
1. indroduce the types streamsize and streamoff
2. mention that basic_streambuf is the component which represent a connection to a data source/sink
3. explain that streams represent the formatting layer of standard i/o, and that each stream contains a pointer to a streambuf to which its i/o functions and operators delegate.
Yup, you should definitely explain 3.
I can then explain that streambuf_facade is the principle component provided by the library, that stream_facade and filtering streams are really only convenience wrappers, but that the tutorial and examples will mostly use the wrappers since these are more familiar to existing users of the standard library and will probably be used most in practice.
How does that sound?
That sounds fine ;)
-------------------------------------------
* 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)
I'm no expert on Boost.Build -- I got the Jamfile to work on my system by trial and error.
unfortunately most of us (including myself) do so. That's just one of the reasons I don't like it (bjam).
Could you explain why you added the lines
<debug><runtime-link>static <release><runtime-link>static
It makes it possible to create a static runtime library, which you can later link an executable against (otherwise, it would be an executable dependent on a DLL or so). I know this is a requirement for MSVC, not sure about other compilers though. Best, John -- John Torjo Freelancer -- john@torjo.com 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)
From: John Torjo <john.lists@torjo.com>
Yes. Thanks. The current name is 'filtering_streambuf'; in the past I've used 'filtered_streambuf' and 'filter_streambuf'. What's your preference?
filtering_streambuf is better
It isn't the streambuf that's doing the filtering; it's the filters pushed onto it. Therefore, I think it should be "filtered_streambuf" or "filterable_streambuf." -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
From: "Jonathan Turkanis" <technews@kangaroologic.com>
"John Torjo" <john.lists@torjo.com> wrote in message:
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.
Many of "us" are happy using streambufs to do work and use streams only when we want the formatting. Still, pushing the streambuf discussion into an Advanced Topics section wouldn't be so bad, so long as there was a brief description pointing folks directly to it to get information on streambufs.
I think I need to add -- very early in the documentation -- a brief overview of the standard iostreams library, in which I
1. indroduce the types streamsize and streamoff
2. mention that basic_streambuf is the component which represent a connection to a data source/sink
3. explain that streams represent the formatting layer of standard i/o, and that each stream contains a pointer to a streambuf to which its i/o functions and operators delegate.
I can then explain that streambuf_facade is the principle component provided by the library, that stream_facade and filtering streams are really only convenience wrappers, but that the tutorial and examples will mostly use the wrappers since these are more familiar to existing users of the standard library and will probably be used most in practice.
If you can write the documentation around streams and defer streambufs to an Advanced Topics section, it might reduce confusion, will reduce main-line documentation complexity, and should make the library more approachable. Either way, what you've proposed would be terrific. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;
"Rob Stewart" <stewart@sig.com> wrote in message
If you can write the documentation around streams and defer streambufs to an Advanced Topics section, it might reduce confusion, will reduce main-line documentation complexity, and should make the library more approachable.
Either way, what you've proposed would be terrific.
Good. Thanks. Best Regards. Jonathan
participants (4)
-
Carlo Wood -
John Torjo -
Jonathan Turkanis -
Rob Stewart