[boost] Re: Review of Daryle Walker's "More IO" library beginstoday, August 21, 2004.

From: Jonathan Turkanis (technews_at_[hidden])
Date: 2004-08-28 15:05:32

"Daryle Walker" <darylew_at_[hidden]> wrote in message
news:BD55AC50.E831%darylew_at_hotmail.com...
> On 8/26/04 1:38 AM, "Jonathan Turkanis" <technews_at_[hidden]> wrote:

> >
> > <boost/io/streambuf_wrapping.hpp>:
> >
> > I vote to reject, for the following reasons.

> > However, if one has written a custom stream buffer, the name
> > basic_wrapping_xstream is not what most people want for the matching
> > streams. If the streambuf is called mapped_filebuf, a matching i/o
> > stream might be named mapped_fstream. This could be solved with
> > template aliases:

> > The problem with this is that one has to write new constructors, since
> > they are not inherited. If one wants to follow the example of the
> > standard library file and string streams, one should also repeat the
> > typedefs char_type, traits_type, pos_type, etc. As a result, the above
> > is scarcely easier than writing mapped_fstream from scratch, since
> > writing the constructors (and typedefs) is the principle difficulty.
>
> You didn't notice the extra "rdbuf" and "is_using_internal_streambuf" helper
> member functions?  I admit that the examples basically are doing the
> repetition you're complaining about.  But, it's better than nothing, right?

Only marginally, I think. And for such a small benefit, I'm not sure it's worth
making someone reading the code familiarize himself with a new template.

> > Without template aliases, a macro-based approach would be more useful:
> >
> >   #define BOOST_IO_DEFINE_WRAPPING_ISTREAM(stream, streambuf, arity)
> >   #define BOOST_IO_DEFINE_WRAPPING_OSTREAM(stream, streambuf, arity)
> >   #define BOOST_IO_DEFINE_WRAPPING_IOSTREAM(stream, streambuf,
> > arity)
> >
> >   BOOST_IO_DEFINE_WRAPPING_ISTREAM(mapped_fstream, mapped_filebuf,
> > 1)
>
> You generally can't do a blind wrapping.  Doing that prevents you from
> passing along the extra member functions from the stream buffer to the
> stream class.

 Good point. I guess this show my biad in favor of keeping a lean
open/is_open/close interface. But in general, you are right that more members
may be needed.

> > Also, let me note that with my library, streams and stream buffers
> > are generated simultaneously using stream_facade and
> > streambuf_facade, so you will rarely need to write a stream buffer
> > from scratch and wrap it in a stream. E.g.
> >
> >   struct tcp_resource { ... };
> >   typedef streambuf_facade<tcp_resource> tcp_streambuf;
> >   typedef stream_facade<tcp_resource> tcp_stream;
>
> Your framework and my wrappers work at different levels.  Your framework is
> all about turning sources/sinks/filters into stream buffers.  My wrappers
> only do a stream buffer to stream conversion.  You have a stream facility
> too, but I guess it just does the main stream buffer conversion followed by
> a wrapping scheme similar to mine.

Yes, it's almost identical. It even uses base_from_member :-)

> > I vote to reject, for the following reasons.
> >
> > 1. std::stringstream already writes to an internally managed character
> > array, but it suffers from the following limitations (both resaonable
> > in context)
> >
> > a. You can't define a stringstream to access a region of memory
> > specified in advance
> > b. when you finish performing i/o, you can't get direct access to
> > the underlying array.
> >
> > array_stream addresses b but not a, and doesn't address b very well,
> > since the lifetime of the array is included in the lifetime of the
> > stream. I believe the pointer streams are better in every respect than
> > the array streams.
>
> There are times where you don't need the character buffer to be around
> outside the stream's lifetime.  (The std::stringstream class has a similar
> limitation, except you can copy out the string easier.)  In those cases, why
> bother allocating an array object outside the stream?

Just to avoid having too many library components with overlapping functionality.
You yourself say that array_stream is 'fairly uninteresting', but that it 'may
have' real-life applications. Some reviewers found this rationale lacking last
year. The fact that you still don't have a better rationale tells me that it
should be rejected.

> > 2. basic_array_streambuf should be implemented as a thin wrapper
> > around basic_pointerbuf. Otherwise, you've got unnecessary code bloat,
> > since all the code is duplicated for each value of N.
>
> That's the inherent trade-off using compile-time arrays.

There's no trade-off, that I see. Using a wrapper around pointerbuf reduces code
bloat but should have no adverse performance impact. Right?

> > ----------------------------------------
> >
> > <boost/io/pointer_stream.hpp>:
> >
> > I think pointer streams and stream buffers are moderately useful, and
> > would vote for acceptance, but I have the following objections.
> >
> > 1. Having separate classes for const and non-const pointers leads to
> > far too many templates. Const-correctness can be enforced by
> > the stream buffer by not allowing constructors taking const char* to
> > set the put area.
>
> I deliberately didn't want that.  The old std::strstream class has at least
> three distinct modes.  It can act like std::stringstream, like
> pointerstream, or like iconstpointerstream.  And I think it can sometimes
> switch between those behaviors in mid-use.  I wanted separate classes for
> those uses, especially after the Standard only included one separation.

I know. The problem with strstream is that it does too much. It's good to get
rid of allocation functions. But the didea of a stream which can be opened in
read-only, write-only or read-write mode is very familiar. It's much better than
multiplying the number of templates by 2.

> > 2. The interface is strange and poorly documented. For example, the
> > specification of begin_pointer and end_pointer refer to the 'utilized
> > array-segment'. What does this mean? Looking at the source, I see that
> > they return pointer delimiting the get area, if the get area is valid,
> > and pointers delimiting the put area otherwise. Why not simply make
> > them return the pointers passed to the constructor, and call them
> > begin and end? Together with the current seek positions -- available
> > using pubseekoff -- this should give the user all the necessary
> > information in a less confusing way.
> [TRUNCATE]
>
> The "utilized array-segment" is the begin-end pair of pointers you passed in
> as the buffer.  That pointer pair has to be either the get area, the put
> area, or both.  That's why the functions returning the bounding pointers
> look in those areas; storing two extra pointer members would have been a
> waste of space.

I didn't say to store the pointers. Aha ... I see ... you're implementation is
just the natural way to recover the pointers, right?

> I don't call the functions "begin" or "end" because I don't
> want the stream to be directly useable as a STL-like container.

> The "?count" member functions can return the seek positions in a much faster
> way.

Faster because there's no virtual function call?

Jonathan