From: "Jeff Garland" <jeff@crystalclearsoftware.com>
We are extending the IOStreams review for one week until 9/11/04. While we have had many good reviews, the library is quite large and we would like to see addtional discussion and review.
I apologize for not getting my review in by 9/11, but I simply couldn't finish it by then. Even now, I cut things short. Nevertheless, here is my review. There are many comments below, but I'll provide the executive summary first. _________________________________________________________________ Should the library be accepted into Boost? Absent Daryle Walker's strong comments against this library, I'd have said, "Absolutely." Given those comments, I must be more guarded. The MoreIO library did less to simplify creating and managing streams and the cognitive complexity thereof, and it does not provide the filtering that the Iostreams library offers. Still, MoreIO offers some facilities and an approach that leads to questions regarding whether these two are competing or complementary libraries. While more investigation regarding the strengths and synergies of the two libraries seems warranted, it may be best to accept both and sort out the matter later. _______________________________ Design Evaluation The library is well conceived and executed. The design seems sound. There are still questions regarding the overlap and differences between this library and MoreIO. There are fundamental, philosophical differences between the libraries. Use cases should be conceived and compared to see how the libraries can handle each. Only then will we know whether they are complementary or competing and, if the latter is the case, which proves superior according to various criteria including ease of use and performance. It may well be that the stream[buf] parts of MoreIO should simply be added to, even integrated with, this library. _______________________________ Implementation Evaluation I did not have the time to study the implementation. _______________________________ Documentation Evaluation The documentation is astoundingly thorough and quite clear overall. Many have already made suggestions for improvement, but those can only be seen as making good better. I have included lengthy comments on the documentation, but I could not even visit every page! _______________________________ Potential Usefulness Evaluation This library makes creating streams and streambufs accessible to many more programmers than does the Standard IOStreams Library. This means that programmers will be able to create useful components they otherwise might avoid creating. This means the library should prove extremely useful. _______________________________ Miscellaneous I did not have the time to use the library. My review was based soley upon the documentation and some inspection of the code. I spent many hours reading the documentation (much to my wife's chagrin!). I have written manipulators and a few streambufs over the years, but I'm not truly expert with the inner world of streams and streambufs. _________________________________________________________________ Library Name I agree with the sentiment of other reviewers in that the library is misnamed. Since this library provides filtering, complete with chaining, as well as to make it easy to create streambufs and streams, the current name fails to capture all of these features. Since the Standard Library provides streambufs, streams, manipulators, etc., and yet it is called the IOStreams Library, perhaps there's nothing wrong with simply calling this library Boost.IOStreams. (I do think this library should use the same capitalization: "IOStreams" not "Iostreams.") _________________________________________________________________ Design Comments _______________________________ Miscellaneous Wouldn't the InoutResource concept be better named "BidirectionalResource," though that is longer? (Same for InoutFilter.) "Peekable" does not imply being able to put back a character. Wouldn't "Undoable" be a better name for this optional behavior? The normal template parameter abbreviation for a character type is "Char." I don't think abbreviating it to "Ch" is helpful. There's no information provided on the performance of this library. How much overhead does it add compared to custom solutions? For example, how much memory and runtime overhead is there wrt MoreIO's mechanisms? How much overhead is there wrt a custom filtering solution? These are answers users of the library need to judge whether it is sufficient or must they choose another approach. The filter chain ordering for input versus output is awkward. It should always be from the beginning to end; let the library reverse the order. _______________________________ boost::io::converter "converter" isn't well-named. It only widens characters for the resources it converts, so it should be called something else. So that begs the new name. I thought of "widener," but while it's correct English, it's also odd sounding, at least to my ear. "character_widener," would be a little more explicit. Perhaps "expander" or "character_expander" would work. Can converter be used with a wide character resource? Imagine a generic "widen" which ensures that the resulting streambuf or stream uses wide characters. convert's "Int" template parameter is poorly named. "Int" is, of course, a common abbreviation for "integer" in C++. I suggest using "Intern" or even "Internal." The name of "IntTr" should follow suit. _______________________________ basic_newline_filter I realize that this is only an example, but there is no apparent protection from misconfiguring the constructor flags. Grouping the related options into enumerated types and taking separate parameters for each group would make it safer to use. (You can overload the bitwise OR operator to permit combining them conveniently.) That is, since print_CR, print_LF, and print_CRLF are mutually exclusive, they should be part of an enumerated type that does not permit bitwise OR'ing. Since the posix, mac, and windows values are meant to supplant all of the other values, they should be part of their own enumerated type and should be used in a separate constructor. The remaining options can be part of a third enumerated type, with bitwise OR support, that constitutes the second argument to the existing constructor. The ignore_CR and ignore_LF values are poorly named. I suggest ignore_extra_CRs and ignore_extra_LFs. The print_CR, print_CRLF, and print_LF values would be better if named write_CR, write_CRLF, and write_LF, since no printing is occuring, at least in general. _________________________________________________________________ boost::io::reverse Isn't the direction of a filter independent of the filtering? That is, why isn't a filter's interface based upon these semantics: char_type filter(char_type ch); streamsize filter(char const * input, streamsize n, char const * output); Then, the source and destination of the data is under the control of the filter() function and means that whether the filtering is used on input or output is an orthogonal decision. This interface should be every bit as efficient as what you've specified, but makes filters more straightforward and flexible. _________________________________________________________________ Documentation Comments In case it makes a difference, I was reading the documentation at http://home.comcast.net/~jturkanis/iostreams/libs/io/doc/ on 4 SEP 2004 rather than downloaded documentation. _______________________________ General "boiler plate" is one word; I'm guessing it will appear in various places throughout the documentation, so I'm calling it out at the top. When formatting source code may I suggest not indenting twice for braces? Doing so pushes code too far right and reduces readability due to long lines. (This occurs on tutorial.html and read.html, for example.) When referring to concepts, always capitalize them. Thus, "filter" becomes "Filter" in most uses, for example. In many places you mention that something works with a Source or a "standard input stream or a standard stream buffer" or similar. Wouldn't it be better to clearly define once that std::basic_istream models Source, std::basic_ostream models Sink, std::basic_iostream and std::basic_streambuf model both? Then you need never mention "standard xxx stream or stream buffer" again. Otherwise, I'd prefer that "standard input stream" be spelled "std::basic_istream," that "standard stream buffer" be spelled "std::basic_streambuf," etc. _________________________________________________________________ menu.html _______________________________ Contents This is confusing. Apparently, the leading "+" means a different page, whereas entries without the "+" are on the current page, but there is no legend explaining that. Why do some pages, like tutorial.html have a mini-TOC at the top, whereas others, like home.html, which may be the only unique one, get their sections listed in the Contents frame? I'd prefer to see all of the pages handled the same. Put another way, remove "Conventions" and "Acknowledgments" from Contents; they can be found by looking at home.html. _________________________________________________________________ home.html _______________________________ Overview "In addition to providing an abstract framework the library ...in their own right. These include components...bzip2 formats." This is difficult to read due to lack of punctuation and other problems. I suggest rephrasing as: In addition to providing an abstract framework, the library provides a number of filters, sources, and sinks, which also serve as examples of how to use the library. These classes provide access to memory mapped files, file descriptor-based file access, code conversion, text filtering using regular expressions, line-ending conversions, and (de)compression for the zlib, gzip, bzip2 formats. Filtering Streams and Stream Buffers "For filtering data the Iostreams Library provides the templates ^ add comma filtering_streambuf and filtering_stream. Instances of filtering_streambuf or filtering_stream contains chains of ^^^^^^^^ subject/verb agreement: change to "contain" _______________________________ Conventions The second bullet would be easier to follow if you broke each piece into a separate bullet: * Specializations of std::basic_istream will be refered to as standard input streams * Specializations of std::basic_ostream as standard output streams etc. I don't think you should "omit the qualifier _standard_ for brevity." Otherwise, your "conventions" aren't. There's nothing wrong with what you've done, but why not use the phrase, "standard streambuf" instead of "standard stream buffer?" It's shorter and in keeping with what anyone with some familiarity of Standard IOStreams would expect. _______________________________ Footnotes I don't see any text that references the footnotes at the bottom of the page, so their presence is confusing. (Further reading reveals that these footnotes appear at the bottom of every page, regardless of whether they are referenced. May I suggest an endnotes page to which all references point. The notes won't be at the foot of the pages which reference them, but they won't appear on pages that don't. _________________________________________________________________ tutorial.html _______________________________ Defining Streams and Stream Buffers for a New Data Source In discussing the hypothetical read() function, you state that, "If an integer less than n is returned, the end of the stream has been reached." What about sources for which more characters will arrive but are not presently available? " The Iostreams Library allows users to create a standard input stream or stream buffer by defining a classes with a single ^^^^^^^ class "Here source is a convenience base class which provides several member types as well as default implementations of several member functions. (The member functions are not needed here.)" The parenthetical sentence is a little confusing. I suggest rewording like this: Here, source is a convenience base class which provides several member types as well as default implementations of several member functions which aren't used in this example. "We can also define a standard input stream:" I suggest italicizing "stream" and rephrasing like the following, for clarity: We can also define a standard input <I>stream</I> using random_source: " Both streambuf_facade and stream_facade have constructors and overloads of open which forward their arguments to filter or ^^^^^^^^^ to the filter resource constructor. Therefore, the first example could have ^^^^^ previous (That change will avoid making the reader count examples and wonder which code blocks are examples, and which are not. Alternatively, should this sort of reference appear elsewhere, you might consider numbering the examples in a caption so they're easy to spot and call out by number.) _______________________________ Defining Streams and Stream Buffers for a New Data Sink " The case is similar if we wish to define a standard stream buffer to write to a new type of output device." That's worded a little awkwardly. I suggest this: If we wish to define a standard stream buffer to write to a new type of output device, the required functionality is similar to that for creating a new data source. "Here sink is a convenience base class which provides several member types as well as default implementations of several member functions. (The member functions are not needed here.)" The parenthetical sentence is a little confusing. I suggest rewording like this: Here, sink is a convenience base class which provides several member types as well as default implementations of several member functions which aren't used in this example. "We can also define a standard output stream:" I suggest italicizing "stream" and rephrasing like the following, for clarity: We can also define a standard output <I>stream</I> using vector_sink: _______________________________ Filtering Input "...from a provided Source and modifies it before returning it to the user:" ^^^^ caller " Here input_filter is a convenience base class which provides several member types as well as default implementations of several member functions. (The member functions are not needed here.)" The parenthetical sentence is a little confusing. I suggest rewording like this: Here, input_filter is a convenience base class which provides several member types as well as default implementations of several member functions which aren't used in this example. "The function boost::io::get reads a character from an arbitrary Source; it is provided by the Iostreams Library to allow all Sources to be accessed with a single interface." That's worded awkwardly. I suggest rephrasing like this: The function boost::io::get reads a character from an arbitrary Source. This is possible because the Iostreams Library provides the same interface for all Sources. "The last item added to the chain could be any Source." This is the first time you've used the term, "chain," so this is confusing as written. I suggest the following: Notice that push is overloaded to accept both filters and sources (an alphabetic_input_filter and std::cin, in this case). When you push multiple items (one or more filters and possibly a source), you form a "chain." A source is only valid as the last item in a chain. In the following example, reproduced here, the order seems backward: filtering_streambuf<input> in; in.push(alphabetic_input_filter()); in.push(random_source()); If that's the correct order, then you need to explain, even here in the tutorial, why the order is as shown. "We could also add any number of InputFilters to the chain before adding the Source." Since "we" aren't doing this, it would read better if written like this: Any number of InputFilters can be added to the chain before adding the Sink. "They could also use the following convenience typedefs:" I'm not sure what the antecedent to "they" was supposed to be, but I suggest rephrasing that like this: The following typedefs can make using the foregoing types easier: _______________________________ Filtering Output I suggest that your example convert text to lower case as this will eliminate language concerns about being able to map a single lower case character to multiple upper case characters. " Here output_filter is a convenience base class which provides several member types as well as default implementations of several member functions. (The member functions are not needed here.)" The parenthetical sentence is a little confusing. I suggest rewording like this: Here, output_filter is a convenience base class which provides several member types as well as default implementations of several member functions which aren't used in this example. "The function boost::io::put writes a character to an arbitrary Sink; it is provided by the Iostreams Library to allow all Sinks to be accessed with a single interface." That would read better like this: The function boost::io::put writes a character to an arbitrary Sink. All Iostreams Library Sinks use the same interface. "We could also add any number of OutputFilters to the chain before adding the Sink." Since "we" aren't doing this, it would read better if written like this: Any number of OutputFilters can be added to the chain before adding the Sink. "They could also use the following convenience typedefs:" I'm not sure what the antecedent to "they" was supposed to be, but I suggest rephrasing that like this: The following typedefs can make using the foregoing types easier: _______________________________ Efficient Filtering: Buffered Filters "In such cases, each time a character is read from an ordinary InputFilter one incurs the overhead of a call to its member function get." You've changed from "we" to "one" and it sounds odd. Try this: In such cases, reading a character from an ordinary InputFilter means calling its get member function, thus incurring the overhead of a (virtual?) function call. "To avoid this overhead, the library allows users to define Buffered filters which can process several characters at once by implementing a member function read. This function will typically be called only when the buffer associated with the filter by the Iostreams Library becomes full." The reference to "users" instead of "us" sounds odd, and the second sentence is phrased awkwardly. To avoid this overhead, we can create a Buffered filtered which can process several characters at once by implementing a read member function in lieu of the get member function. The Iostreams Library typically calls read only when the buffer associated with the filter is full. That means that the larger the buffer is, the fewer times read must be called, reducing its overhead. (It isn't clear from the tutorial whether read supplants get or augments it. I assumed the former in writing the above.) "OutputFilters can also be Buffered. For example:" You should mention that this is via the write mf: The Iostreams Library also supports Buffered OutputFilters via the member function write. Here's the Buffered version of the toupper_filter: _________________________________________________________________ examples.html _______________________________ Tab-expanding OutputFilter "This example uses an OutputFilter to replace each tab character in a code excerpt with an appropritate number of spaces." ^^^^^^^^^^^^^^ ^^^^^^^^^^^^ some text appropriate (The "code excerpt" to "text" change reads better since there is no "code excerpt" apparent yet.) _______________________________ Line-wrapping OutputFilter "This example uses an OutputFilter to wrap text so that each line is no longer than a specified maximum value. No attention is paid to word boundaries." A little rewording would make this read better: This example...specified maximum value, ignoring word boundaries. _______________________________ Presidential OutputFilter The political humor of this example is unwarranted in Boost documentation. Perhaps you could rename it to "Texas OutputFilter" or "Southern U.S. OutputFilter." There is no description of what the filter actually does in this section. _______________________________ Uncommenting InputFilter "This example uses an InputFilter to remove shell-style..." ^^^^^ Unix shell _______________________________ Regex OutputFilter "This example uses a regex_filter to remove C and C++-style comments from a code excerpt. Although somewhat more ^^^^^^^^^^^^^^^^^^^ omit sophisticated than the Uncommenting InputFilter, above, the ^^^ filter in this example is not adequate for use in production..." ^^^^^^^^^^^^^^^^^^^^^^ this filter _________________________________________________________________ concepts.html Throughout this page, all references to "resource," "filter," etc. should capitalize the words in keeping with their being concepts. _______________________________ Overview "...an InputFilter, which filters input read from a Source, and an OutputFilter, which filters output written to a Sink." I suggest a little rephrasing to make this read better and fill in a couple of gaps: an InputFilter, which filters input read from a Source or another InputFilter, and an OutputFilter, which filters output read from a Source or another OutputFilter, before it is written to a Sink. "InputFilters, OutputFilter and their refinements are called..." ^^^^^^^^^^^^ OutputFilters " In general, a filter or resource may provide access to an input sequence, for reading, an output sequence, for writing, or both." ^ ^ omit both commas "The full sets of requirements for the filter and resource ^^^^^^^ ^^^^^^ ^^^^^^^^ set Filter Resource concepts are summarized in the definitions of the concepts ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Resource and Filter which form the roots of the concept..." ^^^^^^^^^^^^^^^^^^^^^^^^^ omit _______________________________ Resource Concepts For Source, Sink, InoutResource, and SeekableResource, change "Provides" to "Resource providing." "SeekableResource" refers to a read/write "head," but few, if any, Resources for which this library will be used are physical devices with read/write heads. I suggest using "location," "position," or "offset" in lieu of "head" here and in other parts of the documentation. In "SeekableResource," you need a comma between "single" and "repositional." _______________________________ Filter Concepts InputFilter, OutputFilter, InoutFilter, and SeekableFilter refer to a "stream buffer." Shouldn't they refer to Sources and Filters instead? In "SeekableResource," you need a comma between "single" and "repositional." _______________________________ Optional Behavior "Closable: A filter or resource which receives notifications immediately before a stream is closed" "Notifications" is unexpected and you refer to a stream. Try this: Closable: A Filter or Resource which is told to close immediately before a Source or Sink is closed. "Localizable: A filter or resource which receives notifications when the locale of a stream or stream buffer is set using basic_ios::imbue or basic_streambuf::pubimbue." "Notifications" is unexpected and you refer to a stream (buffer). Try this: Localizable: a Filter or Resource which is informed when the locale of a Resource is set using.... In "Direct," you refer to a socket-like interface, but not everyone will understand what you mean. I suggest using the phrase, "function-based interface." _______________________________ Convenience Templates and typedefs ______________ Overview "They ease the task of defining filters and resources...." ^^^^ "simplify" would read better "The template resource and its specializations...new resource types; the template filter and its specializations...new filter types; the template buffered_filter and its specializations...new buffered filter types." Split that into three separate sentences to reduce complexity. "There are also wide-character versions of these templates, resource, wfilter..." ^^^^^^^^ namely wresource _________________________________________________________________ filter.html _______________________________ Description "The class template filter, its subcass wfilter...Library to ^^^^^^^ subclass ease the definitions of new models of the various filter..." ^^^^^^^^^^^^^^^^^^^^^^^ simplify creating "...Library; they are inteded to be used as base..." ^^^^^^^ intended _________________________________________________________________ buffered_filter.html _______________________________ Description "The class template buffered_filter, its subcass ^^^^^^^ subclass buffered_wfilter...Library to ease the definitions of new..." ^^^^^^^^^^^^^^^^^^^^^^^ simplify creating "...Library; they are inteded to be used as base..." ^^^^^^^ intended _________________________________________________________________ modes.html _______________________________ Overview Merge the second paragraph with the first. It's distracting being separated. "Readers new to the Iostreams Library should feel free to ^^^^^^^ ^^^^^^^^^^^^ Users omit concentrate primarly on these two modes." _______________________________ Definitions of the Modes "Whether the reading or writing heads are repositionable, and if so, whether there are separate heads for reading and writing or a single read/write head." Rewriting without the use of "head" and correcting the position of a comma: Whether the reading or writing locations are repositionaable and, if so, whether the positions for reading and writing are separate. "* Whether a filter or resource is Closable." This should be a single line. Table: - change "Involves a single sequence" to "A single sequence" throughout. - change "Involves a two separate" to "Two separate" _______________________________ Mode Hierchy Diagrams ^^^^^^^ Hierarchy The diagram does not translate well to grayscale printing; there's no apparent difference between the blue and green. Using arrows, like with UML class diagrams, would make the lines more meaningful. _______________________________ Mode Tags "As with standard library iterator category tags, the tag corresponding to a mode is convertible to each of the tags corresponding to modes which the first mode refines." ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ of the modes it refines. "This is useful to help reduce...different of filter types." ^^ omit You should provide or link to examples that illustrate using some of the more common modes. _______________________________ The Metafunction mode You need to include an example and the signature of mode so we know how to use it. _________________________________________________________________ policy_based_streams.html _______________________________ Overview "Instances of the policy class can be associated and ^^^ with and disassociated with an instance of streambuf_facade using..." ^^^^ ^^^^^^^^^^^^^^^^ from a streambuf_facade specialization "The class template stream_facade is a policy-based stream template which derives from one of std::basic_istream, std::basic_ostream and std::basic_iostream depending on the mode of the policy class. As with streambuf_facade, instances of the policy class can by associated and disassociated with an instance of stream_facade using its member functions open and close." I suggest a few changes to that paragraph to make it more readable and to correct a few errors: As a convenience for those needing a stream, the library also provides the class template stream_facade, which is a policy-based stream class template. It derives from std::basic_istream, std::basic_ostream, or std::basic_iostream, depending upon the mode of the policy class. As with streambuf_facade, instances of the policy class can be associated with and dissociated from an instance of stream_facade using.... _______________________________ Reference ______________ Class template streambuf_facade ______ Description "Policy-based stream buffer template with an interface...." ^^^^^^^^ class template ______ Synopsis Using "T" as the name of a policy class template parameter is confusing. "T" is, of course, commonly used for a data type stored in a container and for similar purposes. The closest thing to "T" in streambuf_facade is the "Ch" nested type in the policy class. Change "T" to a more meaningful name. ______ Template Parameters "T - A model of the filter or resource concepts. Specializations of streambuf_facade with filter types are used internally by the Iostreams Library to construct chains of filters and resources. Users of the library never need to specialize streambuf_facade with a filter type." Since only the library uses filter types for "T", why not name "T" "Resource" and let the library worry about using filters for the "Resource" parameter? Also, given that only the library does this, omit the last two sentences or move them to an Advanced Topics section. "Tr - A C++ standard library charatcer traits type ([ISO], ^^^^^^^^^ character 21.1.1) with...the character type Ch of T." ^^^^^^^ T::Ch "Alloc - A C++ standard library allocator...where Ch is the ^^^ character type of T." ^^^^^^^^^^^^^^^^^^^ T::Ch, the character type in T. ______ streambuf_facade::streambuf_facade You have two sections with the same heading; both should fall under the heading "Constructors." Otherwise, "streambuf_facade::streambuf_facade" should be written as "streambuf_facade<T,Tr,Alloc,Mode>::streambuf_facade." ______ The first streambuf_facade::streambuf_facade "Before the instance can be used for i/o, one of its..." ^^^^^^^^ streambuf ______ The second streambuf_facade::streambuf_facade "Constructs a streambuf_facade which is ready to perform i/o, where the parameters have the following interpretations:" ^^^^^^^^^^^^^^^ meaning You don't call out function parameters like this many other places. There's nothing wrong with it, but you should choose to explain them in a paragraph or a list and do that throughout the documentation. "buffer_size - The size of any buffers that need to be allocated" In the description of open(), you mention that a size of zero means that the streambuf is unbuffered. Doesn't that apply here, too? "pback_size - The size of the putback buffer, relevant only..." ^^^^^^^ buffer, if any. This is "The T constructors involved must take all arguments by..." ^^^^^^^^ so invoked ______ streambuf_facade::open ^^ <T,Tr,Alloc,Mode>:: "Assocaites the given instance of T with this instance of ^^^^^^^^^^ Associates streambuf_facade, if there is no such instance currently ^^^^ T associated; otherwise, throws std::ios_base::failure. The second ^^^^^^^^^^ associated with this parameter determines the size of any buffers that need to be allocated; a value of zero indicates that i/o should be unbuffered. The third parameter determines the size of the putback buffer; it is relevant only if Mode is a refinement of ^^^^^^^^^^ buffer, if any. It "Each of these members associates with this instance of streambuf_facade a newly constructed instance of the policy type ^^^^^^^^^^^^^^^^^ new T constructed from the given lists of arguments, if there is no such instance currently associated; otherwise, they throw ^^^^^^^^^^^^^^^^^^^^^ associated with this. Otherwise std::ios_base::failure. The T constructors involved must..." ^^^^^^^^ so invoked ______________ Class template stream_facade ______ Examples Where are the streambuf examples? You said they were the "fundamental components" of the library. ___ Defining a simple ofstream ^^^^^^ Simple _________________________________________________________________ filtering_streams.html _______________________________ Synopsis There are two, identical declarations of boost::io::filtering_streambuf. _________________________________________________________________ adapters.html _______________________________ Overview The example references should be by name ("Reading From an STL Sequence" rather than "Example I.") to make the links more meaningful. "2. Filtering stream and stream buffer constructors, as well as their member functions push, are overloaded...." ^^^^^^^^^^^^^^^^^^^^^ push member functions _______________________________ Examples Do any of these techniques apply to filtering_streambufs? ______________ I. Reading from an STL Sequence ^^^^^^^^^^^^^^^ Reading From ______________ II. Overwriting an STL Sequence ^^^^^^^^^^^^^^^ Overwriting ______________ III. Appending to an STL Sequence ^^^^^^^^^^^^^^^ Appending To "A stream which appends characters to a sequence, say std::vector, can be defined as follows:" ^^^^^^^^^^^ a std::vector _________________________________________________________________ code_conversion.html _______________________________ Overview "converter: A resource adapter which which takes a resource with ^^^^^^^^^^^ which a narrow character type and produces a resource with wide ^^^^^^^^^ the corresponding wide character type by introducing a layer of code conversion performed using a std::codecvt." ^^^^^^^^^^^^^^^ using "* converting_streambuf: A wide-character stream buffer template having an interface essentially identical to Don't break the paragraph here. filtering_streambuf, with the following additional property: after zero or more wide-character filters have been added to the chain, a sequence of zero or more narrow-character filters and a narrow character resource may be added. A converter is inserted automatically between the final wide-character filter and the first narrow-character filter or resource." Rephrasing that will simplify it and make it clear: ...essentially identical to filtering_streambuf, which automatically inserts a converter between the last wide character filter, if any, and the first narrow character filter or resource. (Any wide character filters must precede the narrow character filters and resource in the chain.) A similar change to the converting_stream paragraph would be appropriate. _________________________________________________________________ convenience.html _______________________________ Overview You mention a convenience function named "copy" but the only function shown is operator <<. _________________________________________________________________ converter.html _______________________________ Description "The class templates converter is...and produces a resource with ^^^^^^^^^ template wide character type" ^^^^ a wide "If a narrow character resource performs input and output using two distinct sequences (see Modes)," ^^^^^^^^^^^ omit; you link to inout directly later in the same sentence. "Otherwise, attempting to spcialize converter results...." ^^^^^^^^^ specialize _______________________________ Reference ______________ Template parameters ^^^^^^^^^^ Parameters ______________ converter::converter Calling a constructor a member, however accurate, is odd. Why not just call it a constructor? "The third parameter determines the size of the buffers or ^^^^^^^ buffer buffers used for code conversion." "The effect of invoking imbue while code conversion...." ^^^^^ Change to use the code typeface ______________ converter::open "Assocaites the given instance of Resource with this instance of converter, if there is no such instance currently associated; otherwise, throws std::ios_base::failure." "Associates" is misspelled, but rephrasing it would work well: Makes a copy of the Resource to be used by the converter. If the converter already has a Resource, it throws std::ios_base::failure. I don't think "associates" leaves out an important detail, suggesting that the Resource is tracked by pointer and that the Resource's lifetime must extend to the next call to close(). "The third parameter determines the size of the buffer or buffers used for code conversion." ^^^ for the ______________ converter::close "Disassociates from this instance of converter any instance of the resource type Resource currently associated with it, calling cleanup functions as appropriate and destroying the associated instance of Resource." This is awkwardly phrased. I suggest: Destroys the converter's current Resource, if any, calling the cleanup functions of any filters and the Resource, as appropriate. I don't think "dissociate" (not "disassociates," BTW) is necessary. _________________________________________________________________ exceptions.html _______________________________ The Boost Iostreams Library ______________ Policy The text is formatted differently than on other pages for this heading level. "...implements the protected virtual memeber functions..." ^^^^^^^ member ______________ Rationale The text is formatted differently than on other pages for this heading level. _______________________________ Exception Safety "1. Resources are freed by calling destructors or close, if appropriate." Don't break the line between "or" and "close." "3.b. ...If the current resource is removed from the chain and a new one added..." ^^^^^^^^^ one is added "Conditions 2. and 3. rely on the specification of the filter and resource concepts. Note that there is no guarantee that a stream's or stream buffer's character sequences are in a consistent state, i.e., that data has not been corrupted, and no way in general to determine the state of these sequences after an exception." After reading this paragraph, I'm left to wonder whether removing and replacing the resource, as described in 3.b, means that the state of the character buffers is well-defined or still unknown as described here. IOW, this paragraph seems to contradict 3.b and should be rephrased to clarify how the state of the buffers can be known and corrected. _______________________________ Acknowledgments "Thanks to Angelika Langer and John Torjo for discussion of exceptions." What does "for discussion of exceptions" mean? Did you have conversations with them on the subject? Then you need to write, "for our discussions on exceptions and exception handling." _______________________________ Footnotes "1...but that "if the exception is rasied due to an error..." ^^^^^^ raised _________________________________________________________________ regex_filter.html _______________________________ Description "The class template basic_regex_filter filters data using regular expressions the Boost Regular Expression Library." ^^^ via the "As unfiltered data is scanned for matches, portions of data ^^^^^^^^^^^ the portions of the which fall between regular expression matches are forwarded unchanged." _______________________________ Reference ______________ basic_regex_filter::formatter "The type of object which a basic_regex_filter uses to...." ^^^ formatter is the "Since Boost.Function objects are implictly constructible from function objects with the correct signature, users of regular expression filters may define their own function objects with the correct signature and pass them to the basic_regex_filter constructor..." This is incomplete. One can write free functions, lambda functions, etc. that satisfy Boost.Function to create formatters. ______________ basic_regex_filter::basic_regex_filter Here's another example of the (apparently) less common bulleted list of parameter descriptions. "replace - A function which will be passed each match_results object in succession and whose return values will be be used as replacement text" Rewording this will make it clearer: A Boost.Function object to which each regular expression match is passed. The return value is used as the filter's result in lieu of the matched text. _______________________________ Example "See Examples." ^^^^^^^^ Regex OutputFilter _________________________________________________________________ one_step_filter.html _______________________________ Description "Its derived classes model DualUseFilter so that it may be used ^^ they? for either input or output." _________________________________________________________________ newline_filter.html _______________________________ Description "The class templates basic_newline_filter is a DualUseFilter ^^^^^^^^^ template which converts between the text file formats used by...." ^^^^^^^ among "Its sole constructor takes an integral flag parameter used to specify the source and target formats." I don't see the value of this here. In the description of the constructor further down the page, this argument is discussed fully. "Note: It is not known if specializations of basic_newline_filter other than basic_newline_filter<char> are useful. If not, the template parameter will be eliminated." Make this a footnote referenced from "Ch" in the Template Parameters section. _______________________________ basic_newline_filter::basic_newline_filter You misspelled "interpreted" in the table several times. _________________________________________________________________ compression.html _______________________________ Acknowlegments "The compression/decompression filters were influences by the..." ^^^^^^^^^^ influenced "Thanks also to Jean-loup Gailly and Mark Adler and to Julian ^^^^^^^^^^^^^^^^^^^^^^ , Mark Adler and Seward for making their excellent librarys available..." ^^^^^^^^ libraries _______________________________ Installation "Alternatively, the binaries can be build from the source code." ^^^^^ built _________________________________________________________________ file_based_resources.html _______________________________ Overview "...access to memory-mapped files on Windows an POSIX." ^^ and _______________________________ Acknowledgments "The memory-mapped file resources are based on work of Craig..." ^^ on the _________________________________________________________________ file.html _______________________________ Reference ______________ Class template basic_file_source ^^^^^^^^ Template ______ basic_file_source::basic_file_source "Constructs...std::basic_filebuf buf opened as follows:" ^^^ but ______________ Class template basic_file_sink ^^^^^^^^ Template ______ basic_file_sink::basic_file_sink "Constructs...std::basic_filebuf buf opened as follows:" ^^^ but ______________ Class template basic_file_resource ^^^^^^^^ Template ______ basic_file_resource_::basic_file_resource ^^^^^^^^^ resource "Constructs...std::basic_filebuf buf opened as follows:" ^^^ but _________________________________________________________________ text_processing.html _______________________________ Overview "The...provides two types of filters for test processing:" ^^^^ text _________________________________________________________________ rationale.html _______________________________ Design Decisions ______________ Generic Design "2. The library can be easily extended to handle..." ^^^^^^^^^^^^^^^^^^^^^^ can be extended easily (avoids split infinitive) "reduce code bloat by internally wrapping filters and resources in polymorphic classes with virtual function read, write, ^^^^^^^^ functions etc. without a noticable performance impact." ^^^^^^^^^ noticeable ______________ Chain Interface "If there is a need for it, it can easily be restored." ^^^^^^^^^^^^^^^^^^^^^^ can be restored easily (avoids split infinitive) ______________ Return Type of write "...cannot be written, was adopter for these reasons:" ^^^^^^^ adopted _______________________________ Planned Changes "2. The concept Buffered will be renamed MultiCharacter, since it is more descriptive and Buffered would be better used elsewhere." I don't understand this point. "MultiCharacter" does not convey anything useful. Even an unbuffered filter processes multiple characters. "Buffered" seems to convey the right idea. More detail is needed about where you would prefer to use "Buffered" and more consideration of what to call what is now called "Buffered" is needed. "3. ...The concept of a smart resources, which already..." ^^^^^^^^^^^^^^^^^ a smart resource -OR- smart resources "4. ...This is necessary to accommodate i/o models just as..." ^^^^ such? "1. Adopt the convention that reads and writes always block until at least one character is written." ^^^^^^^ transferred _________________________________________________________________ portability.html _______________________________ Overview "...together with a coresponding Jamfile." ^^^^^^^^^^^^ corresponding _________________________________________________________________ installation.html _______________________________ Overview "4. ...and depend on the non-boost library" ^^^^^ Boost " 5. ...and depend on the non-boost library" ^^^^^ Boost _________________________________________________________________ adapt.html _______________________________ Overview "The overloaded function template adapt takes an OutputIterator, a pair of ForwardIterators or a standard i/o stream or stream buffer and returns a resource." The series is confusing as written; I suggest: The overloaded function template adapt takes an OutputIterator, a pair of ForwardIterators, a std::basic_iostream, or a std::basic_streambuf, and returns a resource. _______________________________ Reference ______________ adapt - Iterator Ranges ______ Template Parameters "Source - A model of ForwardIterator." ^^^^^^ FwdIt ______ Return Value "Returns a Resource...delimited by first and last." ^^^^^^^^^^^^^^^^^^^^^^^^^^^ It is probably well understood by all readers, but it might be better to express this more formally. _________________________________________________________________ back_inserter.html Broken link to <http://home.comcast.net/~jturkanis/iostreams/libs/io/doc/functions/classes/back_inserter.html#synopsis> _________________________________________________________________ close.html _______________________________ Description "This gives filters and resources an opportunity to free resources or reset their states in preparation for a fresh..." ^^^^^^ state _______________________________ When is close invoked? It is premature to discuss the invocations of close() without showing its synopsis. You should put the synopsis just after the Description section. ______________ streambuf_facade and stream_facade This section is a bit confusing. First you say that r.close() calls boost::io::close(r, std::ios_base::in) followed by boost::io::close(r, std::ios_base::out). Then, you say that if "r" controls a single sequence, those calls wind up, in effect at least, calling r.close(). ______________ filtering_streambuf and filtering_stream You used the term "complete" here. I recall it being defined elsewhere, but it would be useful if you had a single point of definition to which you could link whenever you mention it. The sequence of close() calls never mentions buf1. Is there a mistake? If not, some clarification is in order. "...each element receives two closure notifications..." ^^^^^^^^^^^^^^^^^^^^^ calls to close _______________________________ Reference ______________ Synopsis You've used "T" to mean a Resource in one case and a Filter in the other. This is confusing. You should use consistent parameter names among all of the library templates. _________________________________________________________________ copy.html _______________________________ Description "The function template copy reads data from a given model of Source - or from a standard input stream or stream buffer - and writes it to a given model of Sink - or from a standard output stream or stream buffer - until the end of stream is reached." This sentence is confusing due to the number of hypenated phrases. If I've got it right, it should be written like this: The function template copy reads data from a model of Source, which might be a std::basic_istream or std::basic_streambuf, and writes the data to a model of Sink, which might be a std::ostream or std::streambuf, until the end of the input stream. "The size of the temporary buffer used may be supplied..." ^^^^^^^^^^^^^^^^ temporary buffer between the input and output streams _________________________________________________________________ get.html _______________________________ Reference ______________ Description On many pages, "Description" is the topmost section. Here, it's a subsection of "Reference." Why? This section duplicates the information provided in the Overview, so one of them is superfluous. (This applies to many pages.) "Attemps to extract a character from a given instance..." ^^^^^^^ Attempts ______________ Template Parameters "Source - A...or a standard input stream or stream buffer type." ^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^ Why not just write "std::basic_istream" and "std::basic_streambuf?" (This applies to many pages.) ______________ Return Type If the return type is known to be "typename std::char_traits<typename char_type<Source>::type>::int_type" why not just show that as the return type in the synopsis? _________________________________________________________________ imbue.html _______________________________ Description "Alothugh imbue is designed to be overloaded..." ^^^^^^^^ Although _________________________________________________________________ read.html _______________________________ Overview "The first overload.... The second overload...." No overloads have yet been shown, so this is premature. Move these sentences to Reference|Semantics. _______________________________ Example An example is premature when you haven't shown the synopsis or described the parameters. IOW, this section should follow the Reference section. _______________________________ Reference ______________ Description On many pages, "Description" is the topmost section. Here, it's a subsection of "Reference." Why? This section duplicates the information provided in the Overview, so one of them is superfluous. ______________ Synopsis The second overload extends past the screen, depending upon the text size. ______________ Template Parameters "T - For the first overload, a model of Source or a standard input stream or stream buffer type. For the second overload, a model of InputFilter." The fact that you must describe the parameter this way is a clear indication that you need to use better parameter names. How about: template<typename Resource> std::streamsize read(Resource& t, ...); template<typename InputFilter, typename Source> std::streamsize read(InputFilter& t, Source& src, ...); Since the definition of "Source" refers to T's character type, you'll have to make reference to Resource's or InputFilter's character type. With these distinctions in place, the subsequent Semantics sections won't need to replicate the declarations. _________________________________________________________________ reverse.html _______________________________ Description "The function template reverse takes a filter and returns a new filter which performs the same filtering operation as the original filter but which is an OutputFilter if the orginal filter was an InputFilter and and InputFilter if the orginal filter was an OutputFilter. Roughly speaking, it reverses the direction of a given filter." The ideas here are conveyed in the wrong order and why, "roughly speaking?" The function template reverse reverses the filtering direction of a given filter. It turns an InputFilter into an OutputFilter and an OutputFilter into an InputFilter. You mention memory overhead that can be a side effect of using reverse(). Are there performance issues as well? Also, shouldn't that paragraph be set off with "Note:" or "Warning:"? You mention that using reverse may not be suitable for some data streams, but you need to give more guidance regarding why one would want to use reverse(). _________________________________________________________________ seek.html _______________________________ Overview "The function template seek is used by the Iostreams Library to perform random access withing a sequence controlled by a..." ^^^^^^^ within "seek is designed...specialized for resource types external..." ^^^^^^^^ Resource _______________________________ Reference ______________ Template Parameters "...A model of one of the resource concepts which..." ^^^^^^^^ Resource ______________ Function Parameters "way - ..." You refer to std::ios_base::beg twice. The latter should be std::ios_base::end. Also, "way" would be better named "dir" or "direction," don't you think? _________________________________________________________________ write.html The same comments apply to this page as to read.html, above. _________________________________________________________________ io_traits.html _______________________________ Overview "The header <boost/io/categories.hpp> contains category tags for classifying models of the various filter and resource concepts." ^^^^^^ ^^^^^^^^ Filter Resource "The header <boost/io/io_traits.hpp> contains the definitions of the metafunctions char_type and category, used to associate two fundamental types with each model of one the filter or resource concepts:" I'm not quite sure how the end of that sentence should be phrased, but it's confusing as is. Perhaps this conveys the right information clearly: ...used to determine the type of characters processed by, and the category, or capability description, of Filters and Resources. ______________ Character Type This section seems out of place. It should be titled, "char_type," but you haven't introduced that template yet. I suggest that this descriptive text be moved to the Class Template char_type|Description section. ______________ Category This section, too, is out of place. I suggest moving the content to Class Template category|Description. _______________________________ Class Template char_type ______________ Description "Metafunction associating a character type to each filter or resource type." char_type is a metafunction associating a character type with a Filter or Resource type. The result is the character type the Filter or Resource reads and/or writes. (I merged your original with the earlier Character Type section text and rephrased the result.) ______________ Template parameters ^^^^^^^^^^ Parameters "T - A model of one of the filter or resource concepts, or a standard stream or stream buffer" As in other places, capitalize "filter" and "resource" when referring to them as concepts. _______________________________ io_traits::type Where did this come from? I don't understand how io_traits::type is related to char_type (without looking at the code, of course). _______________________________ Class Template category ______________ Description "Metafunction associating a category tag to each filter or resource type." category is a metafunction that associates a category tag with each Filter or Resource type. The category indicates whether the type is a Filter or Resource, what the type's mode is, and what optional behavior the type provides. The category is similar to the iterator_category member of std::iterator_traits. Types which represent categories are called category tags. (I merged your original with the earlier Category section text, and rephrased the result.) _______________________________ Category Tags "...simply define a struct extending the existing tags." ^^^^^^^^^ (multiply) derived from The table doesn't which tags are mutually exclusive. Is that exclusivity enforced? For example, a type cannot be both a Filter and a Resource, so only one of filter_tag and resource_tag may be used to create a legitimate category. Is that enforced? Is a category tag validated in any way? That is, what if a developer specifies neither filter_tag nor resource_tag. Is the category tag still valid, will it produce a meaningful error, or will it wreak havoc? _________________________________________________________________ mode.html _______________________________ Description "Metafunction returning a mode tag corresponding the the mode of a model of one of the filter or resource concepts or of a standard stream or stream buffer." mode is a metafunction that determines the mode type that indicates the mode of the Filter or Resource. The mode can indicate whether the type is usable for input, output, or both, and whether and how the type supports seeking. _________________________________________________________________ filtering_stream.html _______________________________ Description "Standard stream template used for filtering data." filtering_stream is a standard stream template used for filtering data. _______________________________ Reference In the function sections, you don't separate descriptions of template parameters from those of function parameters. While it does make things consume more space, I think "Template Parameters" and "Function Parameters" headings would be helpful. ______________ Template parameters ^^^^^^^^^^ Parameters "Tr - The traits type" You should mention that std::char_traits<Ch> is the default for this parameter (it's in the synopsis, of course, but it bears repeating). You should also document which features of the traits class you need so folks that want to write their own can. (You might capture this information in one place to reference from multiple places.) ______________ filtering_stream::filtering_stream This heading is repeated several times because there are several constructors. It would be better to preface them all with one heading, "Constructors." ______________ filtering_stream::filtering_stream (#2) "t - An instance of T" This description is not helpful. I can see what it is from the synopsis. Perhaps: The initial Filter or Resource in the filtering_stream. "The parameters have the following interpretations:" ^^^^^^^^^^^^^^^ meaning ______________ filtering_stream::filtering_stream (#3) There's no parameter list introductory statement like, "The parameters have the following meaning." The parameter list doesn't describe "t" as did the second. ______________ filtering_stream::filtering_stream (#4) The description leaves me wanting more. IIUC, this creates a complete filtering_stream with no Filters that uses an iterator based Resource. For input, that would mean that only the characters in the range [first, last) are read. For output, that would mean that characters are written through *first++ until first == last. If I've gotten that right, you need to say something like that so no one else must deduce the same information. Also, this filtering_stream would be complete, right? There's no parameter list introductory statement like, "The parameters have the following meaning." In fact, you've been inconsistent in this on many pages. However, adding a "Function Parameters" heading should be sufficient in and of itself. ______________ filtering_stream::push (#1) "T - A CopyConstructible model..." You should reference a footnote that contains the information that follows regarding using boost::ref, etc. (That is, move that information to a footnote and then reference it from here.) You'll need to do this elsewhere, too. "t - An instance of T" Can you describe this better? ______________ filtering_stream::push (#1) Parameter "t" is not in the parameters description list. ______________ filtering_stream::push (#2) The signature of this function is identical to the previous push overload. Thus, you should merge the two in the synopsis and when documenting them. ______________ filtering_stream::push (#3) "Iter - An iterator type...Ch and whose traveral..." ^^^^^^^^ traversal ______________ filtering_stream::pop "If the chain is initially complete, causes..." ^^^^^^^^^^^^^^^^^^^^^ was complete prior to calling pop ______________ filtering_stream::emtpy "Returns true if the underlying chain is empty." ^^^^^^^^ contains no Filters or Resources. ______________ filtering_stream::size "Returns the number of stream buffers in the underlying chain." ^^^^^^^^^^^^^^ Should that be "Filters and the Resource, if any?" ______________ filtering_stream::reset "Clears the underlying chain. If the chain is initially complete, causes" If I understand correctly, that should be worded more like this: If the chain was complete before calling reset, it closes and releases all Filters in the underlying chain. In either case, it closes and releases the Resource, if any. _________________________________________________________________ filtering_streambuf.html Many of the filtering_stream.html comments apply. -- Rob Stewart stewart@sig.com Software Engineer http://www.sig.com Susquehanna International Group, LLP using std::disclaimer;