Subject: Re: [boost] [review][sort] Formal review period for Sort library begins today, November 10, and ends Wednesday, November 19
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2014-11-14 07:31:29
On 10 Nov 2014 at 0:39, Edward Diener wrote:
> - What is your evaluation of the design?
It's okay. I would prefer a design where there is a sorted_view which
if std::copy gives an actually sorted copy. Obviously one would then
specialise that scenario with a more directly optimised
specialisation, and you'd also have the API presented here which
looks like std::sort().
I'd also prefer to see fallback implementations for less than random
iterators, right down to forward only iterators.
> - What is your evaluation of the implementation?
There seems to be an assumption that user supplied overloads and
specialisations e.g. swap() don't throw exceptions. I think that
unwise. BOOST_NOEXCEPT can test if something can throw. Explicit
guarantees of what happens when something does throw needs to be in
the documentation, per API.
There seems to be a general lack of explicit C++ 11 support as well.
In particular, I think all Boost libraries should use inline
namespaces to implement ABI version management.
Implementation isn't capable of dual build as standalone use without
any dependency on Boost. I'd suggest my forthcoming Boost.BindLib
> - What is your evaluation of the documentation?
Lacks exception guarantees, one per API.
Lacks scaling graphs near front so people can quickly evaluate
usefulness compared to std::sort. I believe the author will fix this
> - What is your evaluation of the potential usefulness of the
> - Did you try to use the library? With what compiler? Did you
> have any problems?
> - How much effort did you put into your evaluation? A glance? A
> quick reading? In-depth study?
> - Are you knowledgeable about the problem domain?
Author of a very popular open source indexing algorithm library.
> And finally, every review should attempt to answer this question:
> - Do you think the library should be accepted as a Boost library?
Yes with these conditions:
1. Either change its name to Boost.Spreadsort, or do a much better
job at a general sort library e.g. sorted_view, more than one sort
algorithm not in the STL, more than random access iterator support.
2. Exception safety is not obvious, and explicit exception guarantees
need to be made and stated per API in the documentation.
3. Explicit ABI version management in C++ 11 using inline namespaces.
Forthcoming Boost.BindLib can do this for you, or it can be done by
It would also be nice if this library were also capable of standalone
build without Boost, but this is not a requirement.
-- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/