[boost] Re: IOStreams formal review start

From: Thorsten Ottosen (nesotto_at_[hidden])
Date: 2004-09-02 17:28:44

Hi Jonathan,

here are some general comments.

1. Your documentation is clear´, plenty and very well organized. It must have been a huge work!

2. How is boundcheking applied in this example:

 struct vector_sink : public sink {
        vector_sink(vector<char>& vec) : vec(vec) { }
        void write(const char* s, std::streamsize n)
            {
                vec.insert(vec.end(), s, s + n);
            }

I mean, could't n be too big?

3. The example

  filtering_streambuf<input> in;
    in.push(alphabetic_input_filter());
    in.push(cin);

mentions something about the source comming last. Maybe it would be more explicit to say

in.push_filter(alphabetic_input_filter());
in.push_source(cin);

assuming there is a stakc of both types. If there is only one source, then maybe in.attach_source( cin );

4. in the example

                int   c;
                char* first = s;
                char* last  = s + n;
                while ( first != last &&
                        (c = boost::io::get(src)) != EOF &&
                        isalpha(c) )
                {
                    *first++ = c;
                }

how can get(src) == EOF, but first != last? Is this "double" checking necessary? Could it be avoided with another design?

5 In the example,
           void write(Sink& snk, const char* s, streamsize n)
            {
                while (n-- > 0)
                    boost::io::put(snk, toupper(*s++));
            }

    maybe a comment should say that streamsize is signed.

6. tab_expanding_output_filter.hpp maybe we should ask JK about using the new boost license

7. in presidential_filter_example.cpp :
              (std::streamsize) sub.size() );

should this not be boost::numeric_cast<std::streamsize>( sub.size() ); ? (there is probably more)

8.  in usenet_input_filter.hpp

         dictionary_["AFAIK"] =  "as far as I know";
         dictionary_["HAND"] =   "have a nice day";

maybe boost.assign could make the example even coolor?

9. the old discussion of

      typedef typename char_type<T>::type  char_type;

vs

    typedef typename io_char<T>::type  char_type;

pops up. If there ever is a vote, I  would vote for the last version. Also, io_category<T>::type seems better to avoid
confusion with other category traits.

10.   In example like

    filtering_istream in(adapt(s.begin(), s.end()));
    filtering_istream in(s.begin(), s.end());

it seems that you could remove the iterator pair version and provide a ForwardReadableRange version

11. What is the difference between the two examples in "III. Appending to an STL Sequence" besides one is
      much more clumsy?

here are somereview comments:
======================

What is your evaluation of the design?
----------------------------
It seems very well-though out and crisp.

What is your evaluation of the implementation?
-----------------------------
haven't looked.

What is your evaluation of the documentation?
-----------------------------
puts most other docs to shame.

What is your evaluation of the potential usefulness of the library?
------------------------------
high. I never fully had the time to get into the iostreams framework; it simply seemed to compplicated for a weekend study.
I'm very pleased to see yet another library that seems easy to use, yet very powerful. This library shows why we should love C++!

Did you try to use the library?  With what compiler?  Did you have any problems?
-------------------------------
no.

How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
--------------------------------
to hours walking through the docs.

Are you knowledgeable about the problem domain?
---------------------------------
no.
And finally, every review should answer this question:

Do you think the library should be accepted as a Boost library?
===========================================

yes.

here are some directions I would like to see (but don't require)

1. Use of boost,range instead of iterator pairs
2. possible wierd questions like mine above and others that creep up during the review might form a basis for a FAQ
3. I hope differences and commonalites with Daryle's MoreIo stuff can be solved; perhaps by the two authors in combination.

Great work!

Thorsten