Re: [boost] [Review:Algorithms]

Subject: Re: [boost] [Review:Algorithms]
From: Marshall Clow (mclow.lists_at_[hidden])
Date: 2011-09-26 14:16:06

On Sep 26, 2011, at 10:23 AM, Thorsten Ottosen wrote:

> Hi Marshall,
> 
> Some minor comments based on reading the docs only.
> 
> A. xxx_of_val should be named xxx_of_equal or something IMO, or the name should simply be overloaded.

I'd love to overload the name, but I don't know how to get the compiler to distinguish
between:		any_of ( first, last, predicate ) 
and:			 any_of ( first, last, value )

I will think about "any_of_equal". It doesn't seem right to me at first look - but then again, "any_of_val" is not a great name either.

> B. Documentation for clamp: I dislike the "iff". Just say "if", because this doesn't clash with the case v==lo/hi. Use the names low/high or lower_bound/upper_bound. The "High point" terminology may be replaced with "upper bound" IMO.

Great suggestion - "upper bound" is way better than "high point".

> C. (ordered) I think the cannonical way is to use
> 
>  range_iterator<const R>
> 
> instead of
> 
>  range_const_iterator<R>

Done.

> D. write
> 
>  #include <boost/algorithm/search.hpp>
> 
> instead of
> 
>  Header 'search.hpp'
> 
> (and similar for other names) in the documentation.

They're already referenced as "Header <boost/algorithm/all.hpp>" in the reference section.
Do you think that I should match those, or use "#include <boost/algorithm/all.hpp>" as the section titles?

> 
> E. A small code example on each page would be nice.

Noted.
        https://github.com/mclow/Boost.Algorithm/issues/10

> 
> 
> F. Why is there no range-based versions of Boyer-Moore etc.?

Um - because I didn't get around to it?
        https://github.com/mclow/Boost.Algorithm/issues/11
        
> 
> G. You could consider to present the iterator version and the range version of all algorithms on the same page.

I'll see what I can do here.

> 
> H. I couldn't fine any details about detail::BM_traits<>. Do I need to know about these?

Only in exceptional circumstances.
I tried to mention that in the "implentation" section of the BM search.

Apparently I wasn't clear enough.

> I think this is a useful, minor addition to Boost.
> 
> I vote "yes" for inclusion.
> 

Thanks!

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