Re: [boost] Review Request: Creasing (Sequence Properties)

Subject: Re: [boost] Review Request: Creasing (Sequence Properties)
From: Simonson, Lucanus J (lucanus.j.simonson_at_[hidden])
Date: 2010-01-27 15:24:47

Stewart, Robert wrote:
> Simonson, Lucanus J wrote:
>> Joachim Faulhaber wrote:
>>> 2010/1/24 Grant Erickson <gerickson_at_[hidden]>:
>>> 
>>>> The creasing algorithm templates define four template functions for
>>>> determining the order properties of sequences, specifically:
>>>> 
>>>>    * Increasing
>>>>    * Decreasing
>>>>    * Strictly Increasing
>>>>    * Strictly Decreasing
>>> 
>>> I'd suggest to implement only the latter. This would make your
>>> extension both more minimal and more general.
>> 
>> I'm a little confused why there is such a narrow focus on
>> ordering relationships here.  What seems to be the most
> 
> Why is it confusing?  The OP focused the thread and no one
> generalized it beyond the is_ordered idea. 
> 
>> general solution to this is a generic range adaptor that
>> given a range over elements of type T returns an adapted
>> range over pairs of elements of type T that are adjacent to
>> eachother in the input range.  I'm constantly writing such
>> adaptors to convert polygon vertices to polygon edges as
>> pairs of vertices.  Combine that with a foreach or just a
>> while loop and you can apply the predicate.  We can
> 
> You say that like there's no code involved.

I'm paraphrasing, but given the adaptor and foreach the solution is a one liner:

foreach_with_early_termination(a in pair_range_adapt(input_range)), predicate()); 

>> generalize the adjacent-pair-range-adaptor seperately from
>> the foreach-element-in-a-range-apply-functor algorithm.
> 
> OK.  Doing that doesn't preclude handy tools like is_ordered, however.
> 
>> As it happens we
>> have boost libraries that provide a basis for each of these
>> generally useful constructs.  It seems a little pointless to
>> propose a library to combine them to solve a very specific
>> problem.  Nobody is at a loss as to how to code up something
>> to figure out whether elements in an iterator range are
>> sorted, and nobody would or even should look in boost for
>> such a simple thing.
> 
> If you say so.  I don't happen to agree.  Unless one does such a
> thing with any regularity, one must find a recipe or reinvent it
> again each time.  Your statement seems rather like suggesting that
> std::for_each shouldn't be in the Standard Library because "nobody is
> at a loss as to how to code up something" to do such a loop.    

You can take my statement and apply the falicy of taking to an absurd extreme, but that doesn't prove anything.  In your example for_each is a more general feature than the proposed library.  We could similarly say that for loops are not needed in the language because while loops can always get the job done and nobody is at a loss as to how.  This misses the point of what I was saying.

>> Did the author skip the determining
>> interest step and ask for a review prematurely, or am I
>> missing something about the proposed library?
> 
> Any time a library can provide a useful, robust, *named* solution to
> a common problem, it seems worthwhile.  As could be seen from this
> thread, using std::adjacent_find was not immediately obvious, so
> there is room to write less than optimal solutions, making a Boost
> library solution helpful.  Furthermore, if there are any
> compiler-specific quirks that should arise, the library can account
> for those and user code need not do so.      
> 
> Have I missed something in your concern?  Do you still disagree with
> including such tools in Boost, possibly in addition to your
> generalization?  

Creating a named solution for a problem incurrs a cost in complexity in addition to a benefit.  The benefit of convenience of having a named solution for a problem people face regularly needs to be greater than the cost of learning the name for all the people who need the solution.  In this case I think the proposed library fails the test.  I find it easier to figure out how to code up whether a iterator range is sorted based on the stl alone, or on general boost capabilities than to remember a name, order of arguments and header file to include to get a library solution.  In this particular case all four uses for creasing are covered by std::is_sorted directly with predicate less<T>, greater<T>, less_equal<T> and greater_equal<T>.  If you pass greater_equal<T> to is_sorted it returns true only if all elements in the range are decreasing and there are no duplicates.  Now, I agree that is_sorted(b, e, greater_equal<T>()) is kind of hard to understand as meaning decreasing with no duplicates, rather than write a new library function it would be better to put /*decreasing with no duplicates*/ preceding that line in your code.  So the named solution in this case is to the problem that people don't know to use the existing named solution?  I hardly think adding more named solutions is going to solve that problem.

Regards,
Luke