Re: [Boost-users] [range] questions about documentation and usage

On Sun, Nov 4, 2012 at 1:37 AM, Robert Ramey wrote:

Neil Groves wrote:

...

On Sat, Nov 3, 2012 at 11:29 PM, Robert Ramey wrote:

...

Please demonstrate what you would consider an improvement. I think many of us have tried to obtain something from you to help us. I appreciate that negative criticism is to some extent useful, but you aren't giving me clear enough suggestions for positive corrective action.

I pointed out things that I thought were wrong and asked to what I was missing. The feedback that I got back was that no one else thinks anything is wrong and don't see my point. From my stand point, I'm just pointing out mistakes.I didn't think of it as negative criticism. The feedback I've gotten has convinced me that I didn't get any thing wrong

I'm not sure how you came to that conclusion...

and that in fact my observations point to real mistakes in the documentation

Deficiencies, yes, but I don't think you've pointed out any mistakes.

and perhaps in the library as well. Here are a few constructive suggestions.

a) For each concept in your documentation define a concept checking class. OK you've done this - so far so good.

b) Declare a concept class using the BCCL - you've done this too - so far so good.

c) Declare concept archtypes from reading your documentation. This is described in the Boost Concept library documentation. This archtype class looks like:

template <class T> SinglePassRangeArchType { // examples of "valid expressions" from the documentation go here };

OK First red flag - the "valid expressions" are members of type T so it's not at all clear to me how to fill this in.

If one wants to create archetypes of the various concepts, I would think to read [1] and/or [2] (extending the library to UDTs), both of which are linked from [3], the documentation on the Single Pass Range concept. Is the documentation under [1] and/or [2] unclear? On might think to add something like:

namespace boost { template <class T> boost::range_iterator<X>::type begin(T &t){ return; } } // namespace boost

But that raises a few design questions of it's own. 1) What should go into X? 2) this will match just about anything - which will likely create other design problems.

Right, but I can only think that one might think to do the above based on an incomplete reading of the documentation. It may be a fair criticism of the documentation that this pitfall is exists, but it's difficult to use all parts of any library correctly without reading all relevant parts of the associated documentation. In this particular case, the documentation on the various range concepts, like most (all) documentation on concepts that I can think of, only tells you what you can do with a given range concept, e.g., if you were to write a generic algorithm. A separate topic is creating models of the various concepts, and that's what [1] and [2] describe. Given that we've decided (perhaps too soon) we want to support

boost::range::find(std::vector<int>, 0) we likely want to provide

a.begin() as a valid express and X::iterator as associated types.

That's the option described by [1].

Of course this will ripple to boost::iterator_range so that we'll need template<class T> iterator_range { ... T begin(); ... };

But we already have that. So far so good.

I don't follow. What relevance does iterator_range have here, exactly? d) Each function template documentation page should describe the

concepts of it's template parameters.

the range find function described here:

http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/alg... refers to a type "range_return_value" which I can't find anywhere.

Valid criticism. Go up a couple levels in the documentation and you might find [4].

I'm guessing that the type Value can't be any type but is likely restricted to some type related to SinglePassRangeConcept? I don't know what it is.

A reference to existing documentation on std::find would be the easiest/cheapest way to address such documentation deficiencies; AFAIK, boost::find is implemented directly in terms of std::find. A function template page should look more like this example:

http://rrsd.com/blincubator.com/function/

Note that this includes a small example which can be compiled. Note it should contain the header file to be included.

That's a good model.

e) A type or class template documentation page should have a little bit more information. Take your example:

http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/uti...

which isn't too bad. But it would be helpful if it included the following:

Template parameter. I can guess that the "ForwardTransversalIterator" parameter should model the ForwardTransversalIterator concept of the iterator library. But it should be stated explicitly.

Agreed. And, actually, the documentation states that the template parameter actually need not satisfy the Forward Traversal concept, but then only a subset of the documented interface is available. What that subset is, though, is not explicitly documented, I think :/

Model of: SinglePassRange concept

Valid Expressions (AKA member functions) should state the concepts (type requirements on member function template parameters. For example, if I see:

template< class ForwardRange > iterator_range& operator=( ForwardRange& r );

It's not clear what type I can plug into ForwardRange. Can I use any type which models SinglePassRange like this:

std::vector<int> x; boost::range::iterator_rangestd::vector::iterator v(x);

or do have to use another iterator range as an argument?

I can't tell from reading the documentation.

The documentation on boost::iterator_range is, indeed, incomplete. I think it has been around for quite some time and hasn't seen much love for a while :/

Here is a model for a class template page

http://rrsd.com/blincubator.com/type/

==== More or less unrelated to my questions regarding the documentation there are a couple of other observations I came upon while looking into this.

a) The example from a another post

class my_class {}; namespace boost { const int * begin(my_class & b); const int * end(my_class & e); } // namespace boost void test3(){ my_class x; boost::begin(x); boost::end(x); BOOST_CONCEPT_ASSERT((boost::SinglePassRangeConcept)); // error // above ASSERT Traps even thoug my class matches SinglePassRange according // to the documentatino

Even given your above overloads of boost::begin/boost::end (which is not the correct way to extend a range interface to my_class; see [1] and [2]), Boost.Range doesn't presently deduce the iterator types from the begin/end overloads as, say, an STL implementation would for range-based-for. So there's no way for Boost.Range to know that the associated iterator types of my_class are int*'s. There's really nothing in the SPR concept documentation directly to indicate *how* Boost.Range gets those associated types, so it would seem difficult to assert that my_class models the SPR concept "according to the documentation". boost::range::find(x, 0); // error

// Even the though the assertion above traps, this code compiles }

At first I assumed that the second error was due to failing to include the concept checks in the code for the range::find template. I looked at the code and saw that they were there so I can't explain why the find doesn't trap with the same assert as above.

Sorry, not following. Are you complaining that the boost::find call compiles, and you think it shouldn't (I agree, it probably shouldn't)?

b) I thought that all the stuff was built directly in the boost namespace which I object to. I see it's really built in boost::range (much better) but hoisted up to namespace boost via a wrapper and using declaration. I have to observations about this: 1) I don't think using should every be in any header file.

Why not? Seems like a perfectly legitimate way to control ADL. I know this technique is used for the operator| overloads of the Boost.Range adaptors. If this is used also for boost::find (defined as boost::range::find and brought into the boost namespace by a using declaration), it's likely to avoid boost::find being found by ADL. Whether boost::find should really be boost::range::find is another issue.

2) Any user can hoist those things he wants into the some higher namespace - but he can't go the other way. That is, once he uses your header, he can't undo the "hoisting". If you really want to provide this "help" at least make it optional. I think you could do that by providing an optional header which would hoist all the names.

Just a guess: This hoisting is an implementation detail, only used to control ADL.

...

...

The documentation could always use more examples. I did put these into a separate area in the documentation because I wanted to make them all have tests. The change of layout might not have been optimal and I'll think about this.

The examples I'm refering to are not full blown tutorial type examples (which are helpful but a lot of work to make). But rather 5-15 lines mini-programs written at the time the documentation is written. They are not hard to write when the code is being developed and very helpful to users of the documentation.

Simple examples for the Boost.Range functions should be easy to come by and, agreed, would enhance the documentation. - Jeff [1] http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/ext... [2] http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/ext... [3] http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/concepts/sing... [4] http://www.boost.org/doc/libs/1_51_0/libs/range/doc/html/range/reference/alg...