Re: [Boost-users] [Review] Reminder: Boost.RangeEx review is going on

Subject: Re: [Boost-users] [Review] Reminder: Boost.RangeEx review is going on
From: Alexander (gutenev_at_[hidden])
Date: 2009-03-01 16:34:21

Some issues:

1.
This fails to compile:
#include <boost/thread.hpp>
#include <boost/range.hpp>
#include <boost/range/adaptors.hpp>

int main()
{
}

MSVC8, Windows XP, the beginning of output is:
------ Build started: Project: qq, Configuration: Debug Win32 ------
Compiling...
qq.cpp
c:\boost_1_38_0\include\boost-1_38\boost\range\adaptor\replaced.hpp(116) : 
error C2146: syntax error : missing ';' before identifier 
'make_replaced_range'
c:\boost_1_38_0\include\boost-1_38\boost\range\adaptor\replaced.hpp(116) : 
error C2433: 'replace_range' : 'inline' not permitted on data declarations
c:\boost_1_38_0\include\boost-1_38\boost\range\adaptor\replaced.hpp(116) : 
error C4430: missing type specifier - int assumed. Note: C++ does not 
support default-int

2.
I see indirected adaptor, but I fail to see counted adaptor. With 
Boost.Iterator, we have both counting and indirecting iterator, these two do 
opposite things.

For example, I want to rewrite this with Boost.RangeEx:

#include <numeric>
#include <algorithm>
#include <iostream>
#include <functional>
#include <boost/iterator/counting_iterator.hpp>
int main()
{
  std::cout
    << std::accumulate(
      boost::make_counting_iterator(2),
      boost::make_counting_iterator(7),
      1,
      std::multiplies<int>())
    << std::endl;
  return 0;
}

3.
push_back/push_front
It's not evident from their names, neither from their documentation that 
they would use "c.insert(where, begin, end)" not a loop of push_back.

Suggest clarification in documentation and renaming them to:
insert_back / insert_front.

4.
Documentation - what about performance, exception safety, thread safety ? In 
my opinion the documentation should be explicit in what they are depend on 
the implementation of STL being used and adds no overhead on its own. Each 
algorithm should contain explicit notice which STL algorithm it wraps, and 
it would be convenient to link to SGI STL site.

5.
I heard that some STL implementation fail to take an advantage of a range of 
random iterators to reserve() before range insertion.
Is it true ? Is it possible to fix with Boost.RangeEx ?

=====
Then, my opinion

> What is your evaluation of the design?
Good design.

> What is your evaluation of the documentation?
It has some issues mentioned above.

> What is your evaluation of the potential usefulness of the library?
Very useful. Will reduce a lot of boilerplate code. In particular, it 
provides the most elegant solution of "for-each-value-in-a-map" problem. I 
have not used old Range library, because I have not found it that useful.

> Did you try to use the library? With what compiler? Did you have any
problems?
MSVC8, the problem described above.
Without Boost.Thread included before Boost.Range it seems to work.
Maybe tomorrow (March 2nd) will try on production code.

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

> Are you knowledgeable about the problem domain?
Been writing production code in C++ for 2 years, using STL and Boost a lot. 
Work in a project where all STL containers and most of STL algorithms are 
used, also used Boost.Iterators and Boost.Foreach. I have not used 
Boost.Range before, beause have not found it much useful.

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