[boost] Re: [[e-mail]: Re: Formal Review of theSmartContainers library]

From: Thorsten Ottosen (nesotto_at_[hidden])
Date: 2004-10-06 17:11:16

From: "Ross Boylan" <ross_at_[hidden]>

| On Wed, Oct 06, 2004 at 01:39:55PM +0200, Thorsten Ottosen wrote:

| I realize now I might have been able to get away with smart pointers,
| as I don't think they usually copy the objects they point to.

usually not.

| But
| perhaps they would have gagged on the lack of copy c'tors?

no.

| > this is of course unfortunate. I would like to know if you can actually
| > compile
| > the test files that comes with the library.
| > .
|
| I assume that I can't, as the notice said they didn't work with the
| release version of boost.  I'm using 1.31.

yeah, it should work with that version too after I included the missing files
from 1.32.

|| In terms of the documentation design, there are at least 3 use cases:
| 1) someone seeking initial familiarity with the library
| 2) someone who wants to know about a particular container
| 3) someone trying to decide which exact container to use, when they
| are already familiar with the library in general terms.
|
| I was mostly thinking of 2) in my comments.  If you're very smart, you
| can think of all 3!  For 2), one needs an easy way to get to the
| container in question, and then an easy way to see what it inherits.

yeah, 1 should be covered by an imrpoved tutorial. The inheritance
relationship can be made clearer in the "see also" section.

| > |* Calling this the "smart container" library and then using names like
| > |"ptr_xxx" seems an invitation to confusion.  "smart_xxx" would be more
| > |natural.  At this point, it may be too much trouble and possibility
| > |for error to change the names, even if you agree with this point.
| >
| > Nothing is too late, but I don't agree on these names.
|
| Do you agree that having the containers in the "smart container"
| library called "ptr_xxx" is a bit confusing?

not necessarily. just think of auto_ptr, move_ptr, scoped_ptr, shared_ptr,
shared_array, scoped_array
are all part of what we call smart pointers.

| > |** I found it a bit difficult to find reference information.  First,
| > |the material requires a fair number of clicks to reach.  Second, it
| > |was not clear to me where to look for information that was not in a
| > |given entry.  There are several reasons for this.
| > |
| > |The "see also" heading mixes material you must look at (base classes and
| > |pseudo-base classes such as reversible_smart_container) and classes that
| > |are simply related (e.g. ptr_vector and ptr_array).  So it took me
| > |awhile to find the definition for auto_type used in, e.g., ptr_array.
| >
| > ok, I guess I should seperate "see also" and "alternatives".
|
| How about "inherits from" or "protocol from" instead of "see also"?

"inherits from" sounds ok.

| > |** The unsafe mutating algorithms, in the FAQ now, probably deserves
| > |more prominence, in a discussion (in the main documentation) of what
| > |algorithms are appropriate to use with these containers.  This item
| > |makes me nervous.  First, it sounds as if only an implementor of a
| > |library would know which algorithms actually moved things around by
| > |swapping.  This leaves the average user unsure of which algorithms are
| > |safe.  Second, it seems like asking for trouble for it to be so easy
| > |to cause problems.
| >
| > yeah, if there is one rule in C++, then it must be "if you don't know what
you
| > are doing, don't do it" :-)
| >
| > I promise you that I will expand this discussion to include a list of safe
and
| > unsafe standard algorithms
| > and that debug builds will catch other errors.
|
| Can the safe and unsafe algorithms be know a priori, without knowing
| the implementation of the library?

yes. basically the property that makes a algortihm safe can be scribed in two
ways

1. it swaps elements whenever it moves elements around
2. the result of the algorithm is a permutation of the original range, ie, the
range might be reordered somehow, but all elements are still there

| > |4. Is the indirected predicates the right solution?
| > |It was exactly what I was looking for, so I'm inclined to say yes :)
| >
| > ok, I'm very close to saying that we only need one class, say
| > indirected_fun<Fun>,
| > which could replace all of these functions. Would you consider such a
class
| > harder to use? Eg,
| >
| > instead of
| >
| > ptr_set< T, ptr_greater<T> >
| >
| > you had to write
| >
| > ptr_set< T, indirected_fun< std::greater<T> > >
| That's fine.
|
| I thought if I set ptr_set<T> I automatically got the indirected
| comparison.  Would that be the case under either approach?

yes, the only difference being how *I* specify the default predicate for
operator<.

| Or have I
| forgotten how it works?

no, not at all.

| >
| > |13. Is the shared_ptr idea described in future directions a good idea?
| > |I'm not sure I grasp the concept well enough to comment, but it seems
| > |to me it kind of muddies the waters.  That is, shared pointers are a
| > |different kind of ownership concept than this library, and I suspect
| > |it will be cleaner to keep them separate.  I mean, mostly, cleaner for
| > |users.
| >
| > ok, noted.
| >
| > Let me explain what kind of flexibility we might achieve if this was
possible.
| >
| > 1. once you discover that you want to observe pointers in the container,
you
| > currently
| >     would use a bald pointer hoping all goes well and the element is not
| > removed; if ptr_container<shared_ptr<T>> was
| >    posible, you could just change a typedef to switch between safe/unsafe
| Ah, I think I get it.  You share the pointer, but you still get the
| benefits of automatic indirection.

yes, exactly. moreover, all the unsafe algorithms now becomes safe.

| > 2. you get the indirected interface instead of the normal interface
| >
| > 3. you might switch between deep clone and shallow clone semantics of
| > shared_ptr's just by changing a typedef
| >     to use a different clone_manager.
| >
| Yes, that probably would be worthwhile.

ok.

| One other point: in material you snipped, I advocated returning an
| iterator and throwing an exception if something goes wrong.  As you
| pointed out in earlier correspondence, this actually goes against the
| standard library model, which doesn't throw exceptions in similar
| circumstances.  Since, in general, compatibility is good, that's a
| mark against that approach.  In this situation, my personal urge for
| convenience outweighs it, but you certainly shouldn't break
| compatibility without a conscious judgement.

yeah, I'm not sure if this is somthing that "goes wrong". but not throwing we
get a little more
latitude to do what we sees fit. On the convinience front I think I will
change all
std::pair<iterator,bool> to boost::optional<iterator>, thus allowing

boost::optional<set::iterator> i = set.insert( a_key );
if( !i )
    throw foo();

br

Thorsten