Re: [boost] [Review:Algorithms]

Subject: Re: [boost] [Review:Algorithms]
From: Marshall Clow (mclow.lists_at_[hidden])
Date: 2011-09-30 12:14:18

On Sep 30, 2011, at 4:52 AM, Riccardo Marcangelo wrote:

> 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.

Got it:
        https://github.com/mclow/Boost.Algorithm/issues/10

> 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().

As I've said before, I'm resisting that change because I really dislike the name "is_sorted_until"
However, with each new suggestion for it, my resistance grows weaker.

> 
> 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.

https://github.com/mclow/Boost.Algorithm/issues/7

> 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.

Some of the test programs that I provide dereference the end iterators of strings (oops).
This causes exceptions in MSVC.

I think I've fixed them, but if you're having problems in your own programs as well, I'ld like to see your code.

> 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.

Thanks you for the review!

-- Marshall

Marshall Clow     Idio Software   <mailto:mclow.lists_at_[hidden]>

A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait).
        -- Yu Suzuki