Re: [boost] Is Boost.Range broken?

Subject: Re: [boost] Is Boost.Range broken?
From: Dave Handley (dave_at_[hidden])
Date: 2008-11-23 10:37:45

David Abrahams wrote:

>
> on Sat Nov 22 2008, "Dave Handley" <dave-AT-dah.me.uk> wrote:
>
>> One of the biggest complaints I got when supported boost for my firm
>> was about the documentation.  Generally it just isn't up to the same
>> standard as the code.
>
> I don't see it.  It's my experience that the quality of the code and of
> the documentation are strongly linked; maybe even tautologically linked.
> If you can't tell what something is supposed to do from reading the
> docs, how can its implementation possibly be correct in any meaningful
> sense?

So effectively you are saying that if I am correct in my assertion that 
boost documentation is frequently poor, then it follows that boost itself is 
poor.  I can't say I agree - in general, even for those libraries where I've 
suffered poor documentation, I've still been pretty happy with the library 
(with a few exceptions).

>
> I'm not saying all of Boost is well-documented of course; probably some
> of the code isn't as good as I'd like.  Also, not knowing the speicifics
> of peoples' complaints, it's hard to understand much about what
> constitutes "well-documented" from their point of view.

If I get the chance, I may feed back some of the complaints I've heard. 
Although I haven't had good experiences with this list in the past (this 
thread being a good example).

>
>> This is something that is gradually being fixed, but hearing the
>> complaints from Java programmers moving to using C++ and Boost, it's
>> very hard to argue against them.  Given the quality of the docs, are
>> you surprised when people figure things out from the source?
>
> I guess you're assuming I'll agree that the docs are lacking.

You don't necessarily need to agree.  If the docs in any library are 
lacking, people will go to the source to figure out how to use it.

>
>>>> at the top of iterator_range we see:
>>>>
>>>> /*! \file
>>>>    Defines the \c iterator_class and related functions.
>>>>    \c iterator_range is a simple wrapper of iterator pair idiom. It 
>>>> provides
>>>>    a rich subset of Container interface.
>>>> */
>>>>
>>>> This implies to me that range is trying to look and feel like a 
>>>> container -
>>>> not like an iterator.
>>>
>>> I understand that you drew that conclusion, but IMO it's a huge stretch
>>> to claim that a concept that doesn't even exist for containers
>>> (singularity) should behave in some container-like way for ranges.
>>
>> It's not a huge stretch, it's just something that is useful for
>> generic programming.
>
> That seems like a non-sequitur.  The comment I'm replying to seems to
> claim that Ranges are container-like, and thus their is_singular
> function should behave in a container-like way.  I still maintain that's
> a huge stretch: there's just no a priori container-like behavior for a
> function called is_singular, since there are no singular containers.

At no point have I argued that the is_singular function makes a range more 
container like.  All the other "container-like" functions make range more 
container like.  Like begin(), end(), size(), empty(), operator[], etc. 
None of these functions exist on a standard library iterator.

>
>> Like everything in programming, if we get hung up on principles, we
>> forget that people have to use this code.
>
> I'm not hung up on principles, and am fully aware of the importance of
> usability.  Now that the documentation of the old functionality has been
> revealed, I am also fully confident that there is a library evolution
> bug here.  However, I am not convinced that there is a bug in the
> current design because it isn't sufficiently container-like, and the
> argument about is_singular doesn't do anything to bolster that case IMO
> for the reasons I am stating.

That's not my argument.  See my previous comment above.

>
>
>>> Perhaps.  But did the documentation guarantee that it would work?
>>
>> I believe it did - looking at a quote from Tom elsewhere in the thread.
>
> Agreed.
>
>>> You're free to define models of Range that have a default-constructed
>>> empty state.  Requiring all models of Range to behave that way is
>>> antithetical to the principles of generic programming.
>>
>> And that isn't what I'm arguing for.
>
> Maybe not, but the argument that a supposed model of Range is broken
> because it is not sufficiently container-like is headed in that
> direction.

Not really, there are plenty of libraries around there, specifically 
designed for generic programming, where different entities do different 
things.  That's why the standard defined all the different types of 
iterators for example - random access iterators are different to 
bi-directional iterators in many ways, but are still called iterators.  Just 
because you have some ranges where default construction implies empty, 
doesn't mean all of them have to.

>
>> Tom has proposed (and I think in your first response you in general
>> agreed) that 2 versions of iterator_range would be the best solution
>> here.  I fundamentally agree with that position.
>
> Probably -- and unfortunately -- so.  However I also feel it's important
> to temper peoples' expectations about what Range is, and to prevent any
> moves to make the concept more refined than it should be (and thus rule
> out many currently-valid models of Range).
>
>>>> In general, range sits in a strange middle ground.  It is trying to
>>>> be a clever pair of iterators, and also trying to look and feel like
>>>> a container.
>>>
>>> I disagree.  IMO Range is a pair of iterators You can see that by
>>> looking at all of the models that don't behave in any container-like
>>> way, e.g. std::pair<int*,int*>, and if you read any book on the STL
>>> you'll even see pairs of iterators referred to as "ranges."  The key
>>> feature of a Container that distinguishes it from a Range is its
>>> ownership of the values: when you copy it, the values are copied, too.
>>
>> One feature of some containers is that.  Some containers (not in the
>> STL) don't own the data.
>
> Then you're not talking about the STL container concept
> (http://www.sgi.com/tech/stl/Container.html) and I don't know what you
> *are* talking about when you say "container."

When I say container I mean any container that anyone has written.  I 
usually try (although don't always succeed) to distinguish this from the 
example you are giving above, by talking about standard library containers 
in that context.  I have personally written containers that don't 
necessarily own data.  I don't think containers in general have a single 
"defining feature".  I agree that owning the data is a common feature of 
many containers, but so is the interface they have.  A lot of containers 
have a superset of the interface we see on boost::iterator_range.

>
>> But that point isn't really that important.
>
> Sorry, I do think it's important.

We'll be left disagreeing here then.

>
>> An iterator range is also not an iterator,
>
> Never claimed it was.  It's a range (see
> http://www.sgi.com/tech/stl/Iterators.html)

I know it's a range.  Incidentally, the link you have doesn't define a 
range - it just refers a lot to ranges (and mentions a few properties of 
them).  A range, as I've said before, is something in between a container 
and an iterator.  It has features of both, it also lacks features of both. 
We shouldn't necessarily design singularity into ranges (which is what seems 
to have happened at the moment), since as library writers, we are allowed to 
define whether a particular range has singularity or not.

>
>> as well as not being a container.
>>
>>> It's unfortunate that the original implementation of iterator_range
>>> set up different expectations for you, but AFAICT the only thing that
>>> a Range has in common with a container is that it supplies a begin()
>>> and end() that delimit a sequence of elements.
>>
>> And an empty() and a size()
>
> Okay, fine, but with a different (free function) syntax.

empty() and size() on an iterator_range are both member functions - so I'm 
not sure what you mean here.  Boost::iterator_range also provides front and 
back.

>
>> and a random access function operator[],
>
> random-access iterators provide that too.

Agreed.

>
>> etc. And a comment in the source about being container-like.
>>>
>>>> As such it has properties of both.  Singularity is something really
>>>> reserved for iterators, and therefore, I don't think it is necessarily
>>>> obvious that a range either should or shouldn't have the same iterator
>>>> like singularity features.
>>>
>>> I would find that argument more compelling if there was a "singular"
>>> concept that applied to containers, but there isn't.
>>
>> This is precisely the sort of discussion that comes about with a
>> concept that sits between 2 other concepts.  Iterator_range sits in
>> between iterator and container, which is the whole basis of my
>> argument.  How much like an iterator or like a container it is is open
>> to discussion, but it certainly isn't either one of them.  Singularity
>> is a feature of iterators, not containers - I agree - but that also
>> shouldn't necessarily drive the design of iterator_range, since an
>> iterator_range isn't an iterator.
>
> Right.  Since you want to talk about what should drive the design of
> iterator_range, I'll tell you what's relevant to this discussion:
> iterator_range should represent a range built on iterators as its name
> implies (see the link I provided) and should not make any promises that
> force it to have space or time overhead beyond what a pair of iterators
> costs.

I disagree - what should drive the design of iterator_range is what people 
will find useful.  The link you provided uses a range - it doesn't define 
it.  The library is free to define ranges to have the behaviour that is 
considered useful.  In this case different people are asserting that 
different behaviour is useful - in your case you assert that time and space 
constraints are important, Tom has shown that valid empty ranges are useful. 
The most obvious solution to this is 2 versions of the iterator_range class.

>
> Of course, it may be too late for that if people have been relying on
> the earlier behavior.
>
>> It is; however, what the library intends it to be.
>
> Hm?

That assertion comes from the comment in the code that says iterator_range 
is container-like.

>
>> Clearly, in both documentation and code, the library changed it's mind
>> about what an iterator_range was between 1.34 and 1.35 - that's
>> problematic when people have spent a long time coding off the old
>> functionality.
>
> Agreed.  It's not clear to me what the best solution is at this point.
> Changing the meaning of iterator_range back is going to break some other
> peoples' code.

Agreed.

>
>> Finally, we can discuss detailed semantics of what an iterator_range
>> should be all we like, the core issue here though is that
>> iterator_range changed unannounced between 2 versions of boost.
>
> Well, that's certainly a separate issue, and a real problem, and one
> that needs to be addressed somehow.

Agreed.

>
>> Why can't we come to an agreement that 2 versions of iterator_range
>> would make some sense, and move this discussion forward?
>
> Because I'm not yet convinced that's the way to go.  Maybe the best
> solution is a preprocessor switch that restores the old behavior.
>

In that context you would be telling Tom that because he wants the old 
behaviour in some of his code, he would be banned from using the new 
behaviour elsewhere in his code?  Is that your intention?

Dave