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. B. Documentation for clamb: 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. C. (ordered) I think the cannonical way is to use range_iterator<const R> instead of range_const_iterator<R> D. write #include <boost/algorithm/search.hpp> instead of Header 'search.hpp' (and similar for other names) in the documentation. E. A small code example on each page would be nice. F. Why is there no range-based versions of Boyer-Moore etc.? G. You could consider to present the iterator version and the range version of all algorithms on the same page. H. I couldn't fine any details about detail::BM_traits<>. Do I need to know about these? That's it. I think this is a useful, minor addition to Boost. I vote "yes" for inclusion. regards Thorsten
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@gmail.com> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
Marshall Clow wrote:
On Sep 26, 2011, at 10:23 AM, Thorsten Ottosen wrote:
A. xxx_of_val should be named xxx_of_equal or something IMO, or the name should simply be overloaded.
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.
I think the previous suggestion of "any_equal" is spot on.
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?
They all should be "#include <boost/algorithm/all.hpp>" so one can copy and paste straight from the documentation to a source file. _____ Rob Stewart robert.stewart@sig.com Software Engineer using std::disclaimer; Dev Tools & Components Susquehanna International Group, LLP http://www.sig.com ________________________________ IMPORTANT: The information contained in this email and/or its attachments is confidential. If you are not the intended recipient, please notify the sender immediately by reply and immediately delete this message and all its attachments. Any review, use, reproduction, disclosure or dissemination of this message or any attachment by an unintended recipient is strictly prohibited. Neither this message nor any attachment is intended as or should be construed as an offer, solicitation or recommendation to buy or sell any security or other financial instrument. Neither the sender, his or her employer nor any of their respective affiliates makes any warranties as to the completeness or accuracy of any of the information contained herein or that this message or any of its attachments is free of viruses.
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.
I thought I had suggested any_equal, all_equal, etc. in a previous email. Maybe I forgot to send it :)
On Sep 26, 2011, at 12:10 PM, Andrew Sutton wrote:
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.
I thought I had suggested any_equal, all_equal, etc. in a previous email. Maybe I forgot to send it :)
[ Looks back through the email... ] No, you sent it - I just didn't read it. :-( any_equal it is, then. -- Marshall Marshall Clow Idio Software <mailto:mclow.lists@gmail.com> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
On Sep 26, 2011, at 12:10 PM, Andrew Sutton wrote:
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.
I thought I had suggested any_equal, all_equal, etc. in a previous email. Maybe I forgot to send it :)
Ok - I've changed the names, and pushed the changes to github. https://github.com/mclow/Boost.Algorithm If people are more comfortable downloading zip files, I can make another one of those. -- Marshall Marshall Clow Idio Software <mailto:mclow.lists@gmail.com> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
on Mon Sep 26 2011, Marshall Clow <mclow.lists-AT-gmail.com> wrote:
On Sep 26, 2011, at 12:10 PM, Andrew Sutton wrote:
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.
I thought I had suggested any_equal, all_equal, etc. in a previous email. Maybe I forgot to send it :)
Ok - I've changed the names, and pushed the changes to github. https://github.com/mclow/Boost.Algorithm
You should note that the revision officially under review is this one, though: https://github.com/mclow/Boost.Algorithm/tree/Review
If people are more comfortable downloading zip files, I can make another one of those.
I don't think you need to. Github makes them for you: https://github.com/mclow/Boost.Algorithm/archives/master -- Dave Abrahams BoostPro Computing http://www.boostpro.com
Marshall Clow <mclow.lists <at> gmail.com> writes:
I thought I had suggested any_equal, all_equal, etc. in a previous email. Maybe I forgot to send it :)
Ok - I've changed the names, and pushed the changes to github. https://github.com/mclow/Boost.Algorithm
If people are more comfortable downloading zip files, I can make another one
of those.
I'd like to suggest the following ---------------- template<typename R, typename V> bool all_equal_to ( const R &range, const V &val ); returns true iff each item in the sequence compares *equal to* the value val ---------------- template<typename R> bool all_equal (const R &range); returns true iff each item in the sequence compares *equal* to each other (since the comparison is transitive this can be implemented comparing each element in the sequence to the first one). sequences of size 0 and 1 return true, larger sequences can be implemented as all_equal_to ( range, *begin(range) ); ---------------- I have my own version of all_equal and I find it quite useful. What do you think?
On Sep 28, 2011, at 2:02 AM, dariomt wrote:
Marshall Clow <mclow.lists <at> gmail.com> writes:
I thought I had suggested any_equal, all_equal, etc. in a previous email. Maybe I forgot to send it :)
Ok - I've changed the names, and pushed the changes to github. https://github.com/mclow/Boost.Algorithm
If people are more comfortable downloading zip files, I can make another one
of those.
I'd like to suggest the following
---------------- template<typename R, typename V> bool all_equal_to ( const R &range, const V &val );
returns true iff each item in the sequence compares *equal to* the value val
Already got this, in all.hpp (though I'm changing the name to all_of_equal)
---------------- template<typename R> bool all_equal (const R &range);
I'll put it on the list of things to be (maybe) added in the future. -- Marshall Marshall Clow Idio Software <mailto:mclow.lists@gmail.com> A.D. 1517: Martin Luther nails his 95 Theses to the church door and is promptly moderated down to (-1, Flamebait). -- Yu Suzuki
participants (6)
-
Andrew Sutton -
dariomt -
Dave Abrahams -
Marshall Clow -
Stewart, Robert -
Thorsten Ottosen