Re: [boost] Review of Intrusive library begins today March 12

From: Ion Gaztañaga (igaztanaga_at_[hidden])
Date: 2007-03-20 17:39:36

Hi Stjepan,

Stjepan Rajko wrote:
>>   - What is your evaluation of the documentation?
> 
> The general documentation, tutorials on different tasks and the
> examples are nicely organized and well written.  After reading parts
> of the documentation, I was able to understand what the library was
> for, what its advantages and disadvantages are, and a good enough idea
> of how to use it.
> 
> The reference section is very detailed but hard to navigate.  After
> going to the .hpp reference page, I had to figure out that I need to
> click on any of the class names for more information, and then faced a
> dump of all members.  Some grouping of the member functions according
> to their use (even just using white space to separate them) would be
> nice.  I really appreciated the complexity discussions.

The reference section was generated using Boost 
Quickbook/Boostbook/Doxygen toolkit, and I'm pretty it can be improved, 
but I don't know if what you are asking is possible. Maybe I should just 
write the reference myself instead of using the toolkit, but it's a lot 
of work to do if the reference is good enough. But reviewers have the 
last word.

> Overview page:
> 
> Semantically, a Boost.Intrusivecontainer is similar
> -- space missing Boost.Intrusive<->container

Ok.

> Presenting Boost.Intrusive containers page:
> 
> Non-raw pointers: If the wants to use smart pointers instead of raw pointers
> -- If the _user_(?) wants

Thanks.

> Cloning Boost.Intrusive containers page:
> 
> The second parameter is an function object that will clone value_type
> objects returning a pointer to them.
> -- is _a_ function.  

Ok.

> Although, this sentence struck me as slightly
> confusing - perhaps something like "that will clone value_type objects
> and return a pointer to the clone."?

Ok.

> The second parameter is an function object that will destroy value_type objects.
> -- is _a_ function object

Ok.

> The word concepts seems to be getting more and more of a specific
> meaning in C++... I don't know whether there is another word that
> means the same thing as "concepts" but isn't "concepts", but if you
> can think of one it might be good.

I think concepts is fine.

> std::pair< iterator, bool > insert(value_type & value) ;
> 
> Returns: If the value is not already present inserts it and returns a
> pair containing the iterator to the new value and true. If the value
> is already present returns a pair containing an iterator to the
> already present value and false.
> 
> -- I don't think this is quite correct.  The behavior I observed is
> that the value is inserted if there are no elements equivalent to it
> in the set, according to the comparison function.  Not quite the same
> as "if the value is not already present", I think.

Ok. With intrusive containers "tehere is no equivalent value" should be 
better, because the value itself could be already inserted in the container.

>>   - What is your evaluation of the potential usefulness of the
>>   library?
> 
> Very useful.  I really appreciate the possibilites that it presents
> and the associated advantages, as per the documentation.

Thanks!

>> And finally, every review should answer this question:
>>   - Do you think the library should be accepted as a Boost library?
> 
> Yes.  Really nice work!

Good vote!

Regards,

Ion