[boost] Re: Formal review of "Output Formatters" library begins today

From: Jeff Flinn (TriumphSprint2000_at_[hidden])
Date: 2004-09-12 13:04:55

"John Torjo" <john.lists_at_[hidden]> wrote in message
news:414443E0.60008_at_torjo.com...
> Dear boosters,
>
> The FORMAL Review of "Output Formatter" library begins today,
> Sept 12, 2004.
>
> 1. What is your evaluation of the design?

I find the naming conventions misleading. 'outfmt' but it does io? What is
an '...ob' and '...obex'? This effort at reducing verbosity is then
countered as mentioned by Dave Abrahams, by the "syntactic noise".

I do like the delimiter traits objects approach to dealing with
leader/trailer existence issues. Although I have not found passing "" (empty
string) as the leader/trailer being a performance bottleneck since they are
only performed once per range.

I like the range approach, but isn't a range library coming? Will io::range
be entirely compatible? My home grown range_streamer is often used with
filter and transform iterators. It would be nice to use a shorter syntax
like:

    os << range_stream( aC, lFilterFnc, "\n=\n{ ", "\n, ", "\n}" );

rather than

   s << range_stream( boost::make_filter_iterator( lFilterFnc, aC.begin(),
aC.end() )
                    , boost::make_filter_iterator( lFilterFnc, aC.end  (),
aC.end() )
                    , "\n= { "
                    , "\n  , "
                    , "\n  }"
                    );

> 2. What is your evaluation of the implementation?

The main motivation I had for developing a similar output facility was
dealing with the dangling separator when using std::copy and
ostream_iterator in a consistent/performant fashion.

list.hpp does:

   os << open();

   while( first != last )
   {
      fo.write( os, *first );

      if( ++first != last )
         os << separator();
   }

   return( os << close());

Which does two comparisons for each element in [first,last). This can be
rearranged to do a single comparison as shown below.

   os << open();

   if( first != last )
   {
      fo.write( os, *first );

      for( ++first ; first != last ; ++first )
      {
        os << separator();

        fo.write( os, *first );
      }
   }
   return( os << close());

This is the only recognizably similar code to that which I've bee using.
(Which I'll attach for comparison purposes at the end of this review).

> 3. What is your evaluation of the documentation?

After reading Jonathon's IOStream documentation, my expectations have been
raised. Some of the problems are due to the misleading names used in the
implementation itself. Almost everything is a format_this or format_that
with format methods... Perhaps an additional namespace level would alleviate
some of this.

Missing are Motivation and Rationale sections. When/Why would someone use
this library versus ostream_iterator/Tokenizer/Spirit/...

> 4. What is your evaluation of the potential usefulness of the library?

At least on the output side, I've found this functionality very useful. It
replaced a lot of legacy repeated code within my company where everyone had
their own way of dealing with the issue.

> 5. Did you try to use the library?
>     With what compiler?  Did you have any problems?

No.

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

An hour with the documentation and looking at some of the code.

> 7. Are you knowledgeable about the problem domain?

I've implemented and extensively use a simplified version that does output
only given a pair of iterators and delimiters.

> 8. Do you think the library should be accepted as a Boost library?

Not in current state.

Jeff Flinn
==========

#if !defined(RangeStreamerHeaderIncluded)
    #define  RangeStreamerHeaderIncluded

template< class tItr, class tDBeg, class tDInner, class tDEnd > class
range_streamer
{
   tItr    mBeg;
   tItr    mEnd;
   tDBeg   mDBeg;
   tDInner mDInner;
   tDEnd   mDEnd;
public:
   range_streamer( tItr aBeg, tItr aEnd, tDBeg aDBeg, tDInner aDInner, tDEnd
aDEnd )
   : mBeg   ( aBeg    )
   , mEnd   ( aEnd    )
   , mDBeg  ( aDBeg   )
   , mDInner( aDInner )
   , mDEnd  ( aDEnd   )
   {}

   template< class tStream >
   void OutputTo( tStream& s )const
   {
      if( mBeg != mEnd )
      {
         tItr lItr = mBeg;

         s << mDBeg << *lItr;

         for( ++lItr ; lItr != mEnd ; ++lItr )
         {
            s << mDInner << *lItr;
         }
         s << mDEnd;
      }
   }
};

template< class tItr, class tDBeg, class tDInner, class tDEnd >
inline range_streamer<tItr,tDBeg,tDInner,tDEnd>
range_stream( tItr aBeg, tItr aEnd, tDBeg aDBeg, tDInner aDInner, tDEnd
aDEnd )
{
   return range_streamer<tItr,tDBeg,tDInner,tDEnd>( aBeg, aEnd, aDBeg,
aDInner, aDEnd );
}

template< class tStream, class tItr, class tDBeg, class tDInner, class tDEnd
>
inline tStream& operator<<( tStream& s, const
range_streamer<tItr,tDBeg,tDInner,tDEnd>& aRS )
{
   aRS.OutputTo(s);

   return s;
}

#endif //RangeStreamerHeaderIncluded