[boost] [Review:Algorithms]

Subject: [boost] [Review:Algorithms]
From: Riccardo Marcangelo (ricky.65_at_[hidden])
Date: 2011-09-30 07:52:12

Hi Marshall

Firstly, I think the documentation is quite good. I like that there is a
description of what each algorithm does, its return type and complexity.
Maybe one example of each algorithm would be a nice addition.

I think one_of complements the C++11 algorithms all_of, any_of and none_of
well. is_increasing, is_strictly_increasing, is_decreasing and
is_strictly_decreasing are useful, especially for statistical ranges. These
are rolled fairly often so this is a welcome addition.

is_ordered() seems to be similar to the C++11 algorithm is_sorted_until()
with a comparison function. For consistency I would recommend using the
behaviour of is_sorted_until() and renaming the function to
is_sorted_until(). If this change were made I think there would also be a
strong case for including the C++11 algorithm is_sorted().

I'm a bit sceptical on passing the arguments to clamp() by value, except for
the predicate. I think (actually I'm totally convinced for myself) that
clamp() should be overloaded, one version passing the arguments by const
reference and the other passing the arguments by reference. Otherwise it's
doing unnecessary copying in some cases. If V is large the overhead of the
unneeded copies could be a lot. Another reason to not pass the arguments by
value is that comparing objects should not raise any exceptions but copying
usually can. The return types for the two overloaded versions would be
"const V &" and "V &" respectively.

Sadly, I was unable to use the search algorithms,
boyer_moore_search(),boyer_moore_horspool_search() and
knuth_morris_pratt_search(). All failed with a "string iterators
incompatible" error during runtime using Visual C++ 2010. At first I thought
it was my usage but the examples provided also had this problem.

Overall, I like the library and vote "yes" for it to be included into Boost
although I hope the issues I raised can be addressed.

Kind regards

Riccardo

P.S Sorry if I haven't followed the protocol properly with this review, I'm
fairly new to the Boost mailing list.

--
View this message in context: http://boost.2283326.n4.nabble.com/Review-Algorithms-tp3859590p3859590.html
Sent from the Boost - Dev mailing list archive at Nabble.com.