[boost] Re: [review] Formal review of theSmartContainerlibrarystarts today

From: Thorsten Ottosen (nesotto_at_[hidden])
Date: 2004-09-26 17:15:42

"Pavel Vozenilek" <pavel_vozenilek_at_[hidden]> wrote in message
news:cj7d2c$tta$1_at_sea.gmane.org...
| Hello Thorsten,
|
| > |  - small readable code examples
| >
| > ok, what do you find particular non-readable about all the examples in the
| > example section?
| >
| They should fit into 1/2 of page whenever possible.

yes, I'm sure there will be lots of improvements to the docs before a
post-review.

| > | 7. docs, Introduction:  Assignable and
| > |    Copy Constructible could be hyperlinked.
| >
| > ok, don't people know this?
| >
| Boost authors for sure. Ther are also beginners.

yes, agreed.

| | _____________________________________________________
| > | 14 Clonable concept: would it make any sense to provide
| > |    support for placement new/delete also?
| >
| > I can't answer that question since I have never had the need to use it. I
| hope
| > other reviwers can anser it.
| >
| This feature can be likely accomplished when object
| factories get supported

ok.

|
| _____________________________________________________
| > | 15. Clonable concept implementation:
| > |     shouldn't there be try/catch in:
| > |
| > |     template< class T >
| > |     void delete_clone( const T* t )
| > |     {
| > |         checked_delete( r );
| > |     }
| > |
| > |    to enforce the constraints?
| > |    At least it would be useful to have it
| > |    in debug mode to report violation.
| >
| > but then again, shouldn't it then be in checked_delete()? In some
| > sense I feel it
| > beyond a library to cope with "fools" :-)
| >
| checked_delete accepts 0. smart containers cannot.

yes, so you want

try
{
    if( t == 0 )
      throw something();
   checked_delete( t );
}

? :-)

| > |    These exceptions should be declared in
| > |    <boost/smart_containers_fwd.hpp>.
| >
| > what for?
| >
| it is quite common.
|
| #include <...fwd> // no need for full declaration or to search where
| exceptions are
|
| try {
|    some_func_using_some_smart_container()
| } catch(smart_container_base_exception&) {
| }

yes, ok.

|
|  _____________________________________________________
| > | 65. details/scoped_deleter.hpp: there seems no need
| > |     for scoped_array here (only to bloat executable
| > |     and slower compilation).
| >
| > hm...I need to manage a resource and I prefer not to do so manually.
| > I canøt imagine that scoped_array<> afffects the executable at all.
| >
| There could be performance advantage not to use scoped_array.
| And it is one line in destructor.

yeah, I'll look into it.

| > | _____________________________________________________
| > | 67. It would be nice to support systems without
| > |     enabled exceptions (BOOST_NO_EXCEPTIONS).
| >
| > this is harder than it seems because I would have to check return values
| for
| > 0, right?
| >
| I think no. Failed allocation should terminate application
| in such case.

ok, that makes it easy.

| > what do you suggest?
| >
| Alexandrescu's style?

yes, ok.

| One more question:
|
|  - if I have polymorphic hierarchy, will the allocate_clone(T*)
|    clone the dynamic type of class or its static type?

it's up to you. If you let it return ptr->clone() which is virtual, then its
virtual ortherwise its not.
So in the examples from the toturial, animals are cloned virtually.

| I ask because
|    there's semifinished library (polymorphic_map) in Files section
|    that could be used for such purpose.

I took a look at this stuff. It has AFAICT nothing to do with smart
containers, in ptr_map<key,T>, T will be heap-allocated.
in polymorphic_map<key,T>, key might be heap-allocated. The usage for
poymorphic_map seems to be very narrow
AFAICT.

br

Thorsten