Re: [boost] ptr_container: ptr_map_adapter interface

From: Thorsten Ottosen (tottosen_at_[hidden])
Date: 2006-01-03 12:08:01

axter wrote:

> Please feel free to correct me on any of the items listed below.  Most of
> the items listed below, you'll find relevant comments in the above test
> code.
> 

> 3.	ptr_vector::push_back fails to compile with a de-referenced
> iterator

I originally had this, but during the review there was some objection 
against having these.

> 4.	ptr_vector produces a runtime error when assigning one container to
> another via operator[]

I need to see an as small as possible test that shows this. Given that 
you prohibit slicing (by declaring an explicit copy constructor or by 
removing copyability totally) it might be due to this.

> 5.	Produces multiple compiler warnings when using VC++ 7.1

Yes. Vc7.1 and other compilers warn against far too much. ADL for example.

> 6.	Will fail to clone the right type if a derived-derived type fails to
> implement the clone method.

There is no magic like in shared_ptr here. That would impose some overhead.

> 7.	Does not have the standard vector::assign member function that takes
> (size_type, const Type&)

Might be possible to add, though the second argument might be an 
auto_ptr, a bald pointer or a reference.

I'm considering adding a little more initialization help
in boost.assign:

http://www.boost.org/libs/assign/doc/index.html#ptr_push_back

> 2.	Can not insert using an abstract pointer.

implement new_clone().

> 3.	Can not find using an abstract pointer.

please produce a minimal example after implmenting new_clone().

> 4.	Crashes when inserting via de-referenced iterator from another
> container 

ditto.

> 5.	boost::ptr_set is publicly derived from boost::ptr_set_adapter,
> however ptr_set_adapter does not have a virtual function, nor does any of
> it's base classes.

The classes are not copyable, so you won't run into slicing problems. 
The inheritance is purely for inheriting implementation.

> 6.	Fails to compile when used with ptr_set::const_reverse_iterator
> logic

please produce a minimal example.

> boost::ptr_map
> 1.	Unable to compile with an abstract type

implement new_clone();

> 2.	Does not have an insert type for std::pair

and cannot provide one without allowing memory leaks.

> 3.	Can not insert using a constant for 1st argument

ditto.

> 5.	Does not support the pointer as the key type

right. use shared_ptr or something then.

> 6.	Fails to compile insert to iterator type

please produce minimal example.

> 7.	ptr_map::equal_range does not return std::pair<iterator, iterator>
> type as does the standard map::equal_range

nor does it need to. the two classes are used in vastly different 
domains and so generic code working with them both is an illusion.

> 8.	Does not use the standard first and second iterator members as does
> the standard map::iterator

right, you're not allowed access to the pointer.

> 9.	Instead of using first and second iterator members, boost::ptr_map
> uses operator-> and key() functions to access first and second.  However, it
> still fails to compile when using const_reverse_iterator.

A minimal example is appreciated.

Thanks

Thorsten