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

Subject: Re: [boost] [Review] Reminder: Boost.RangeEx review is going on
From: vicente.botet (vicente.botet_at_[hidden])
Date: 2009-03-02 17:08:42

Neil,

as I have already said excelent library. I was waiting for that on Boost for a long time already.

I like the pipe operator '|' and I think that this can be generalized to other functors other than range adaptors. IMHO this is out of the scope of the Boost.Range library, or in oder words that Boost.Range can profit as other for a generic framework. I don't think the first release of Boost.Range should come with the operators interface and a deeper analisys about how to chain functors must be done in a separated library.

I would like that all the range stuff resides in the range namespace waiting for a consensus on where to place common algorigthms, directly on boost, on boost::algorithm::, on boost;; .... I think that all the common algorithms (in namespace boost::) interface should allow a partial specialization. The library do not address this issue. The same name could be applied to ranges, fusion sequences, .... Free functions should not be included in the boost  namespace if specializations are not considered, but this is only my opinion.

If a function works only with a concept the library should use enable_if to avoid invalid instantations, but this could come later. I realy think that the abandoned ConceptTraits library should be included in Boost.

Respect to the name of the adaptors I prefer the -_view than the  -ed suffix, but I can live with both. It would be good to use the same names for the same thing (i.e. be coherent with other boost libraries as e.g. Boost.Fusion).

Even if the user can implement the standard algorithms _if _copy and _n with the library, I consider that the library must provide them for ranges and let other combinations to the user.

> - What is your evaluation of the design?

The library has a simple and clean design.

> - What is your evaluation of the implementation?

I have no taken too much time to look in, but what I have see is well implemented.
I have just a problem with the use of the concept check using the Boost.ConceptCheck, library. The compiler messages are not enoug expicit. For example:

../../../../../boost_1_38_0/boost/mpl/eval_if.hpp: In instantiation of `boost::mpl::eval_if_c< true, boost::range_const_iterator<A>, boost::range_mutable_iterator<const A> >':
../../../boost/range/iterator.hpp:63:   instantiated from `boost::range_iterator<const A>'
../../../boost/range/difference_type.hpp:26:   instantiated from `boost::range_difference<const A>'
C:\cygwin\home\Vicente\boost\sandbox\interthreads\libs\interthreads\example\parallel_sort2.cpp:56:   instantiated from here
../../../../../boost_1_38_0/boost/mpl/eval_if.hpp:60: error: no type named `type' in `struct boost::range_const_iterator<A>'

> - What is your evaluation of the documentation?

Good enough. I would like to see an example for each one of the algorithms as it is done for the adaptors.

I would like also to see how an adaptor and an algorithms are implemented. This will help to those wanting to propose others.

Just a problem with links: the links form the header table of contents of the adaptors do not point to the expected section.

> - What is your evaluation of the potential usefulness of the library?

Very useful, the range algorithms as the range adaptors.

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

I have used the library with gcc-3.4.4 on cygwin. I haven't found any problem.

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

4-5 hours.

> - Are you knowledgeable about the problem domain?

Yes, I use this kind of lazy mechanism since a log time.
 
> - Do you think the library should be accepted as a Boost library?

Yes, this library must be in Boost. I have no doubt.

Good luck,
Vicente