Boost logo

Boost :

From: Pavel Vozenilek (pavel_vozenilek_at_[hidden])
Date: 2004-09-26 16:41:16


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.

 _____________________________________________________
> | 2. docs: the link to license file may be better local.
>
> ok, why?
>
for those w/o inet access all the time

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

| _____________________________________________________
> | 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

_____________________________________________________
> | 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.

 _____________________________________________________
> | 20. Clonable concept: is there any requirement on
> | *a_object == *a_cloned_object?
>
> I guess the requirement could be tightened to say as above iff
> there is an operator==(). I'm not sure its useful to say so, so why add
it?
>
Probably it isn't useful.

| _____________________________________________________
> | 21. Clonable concept: is there anything similar to
> | std::nothrow?
> |
> | Something as allocate_clone<std::nothrow>(p);
>
> If people want it there could be. However, we don't want 0 pointers in the
> containers, so
> what would be the point of it?
>
You are right.

| _____________________________________________________
> | 22. Clonable concept: would it be possible to add
> | support for object factories? Such thing may
> | be useful e.g. for COM or CORBA objects.
>
> yes, how do they look like?
>
I'll think about details.

> | 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&) {
}

 _____________________________________________________
> | 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.

> | _____________________________________________________
> | 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.

> | _____________________________________________________
> | 68.
> |
> | Likewise in ptr_map_adapter.hpp:
> |
> | typedef BOOST_DEDUCED_TYPENAME
> | smart_container::detail::map_iterator<
> | BOOST_DEDUCED_TYPENAME
Map::const_reverse_iterator,
> | BOOST_DEDUCED_TYPENAME Map::key_type,
> | const value_type>
> | const_reverse_iterator;
> |
> |
> | I have problem
> | to find what
> | is end and what
> | is beginning here
>
> what do you suggest?
>
Alexandrescu's style?

      typedef BOOST_DEDUCED_TYPENAME
      smart_container::detail::map_iterator
      <
          BOOST_DEDUCED_TYPENAME Map::const_reverse_iterator,
          BOOST_DEDUCED_TYPENAME Map::key_type,
          const value_type
> const_reverse_iterator;

> Thanks for your review. Hair-raising as ususal :-)
>
One more question:

 - if I have polymorphic hierarchy, will the allocate_clone(T*)
   clone the dynamic type of class or its static type? I ask because
   there's semifinished library (polymorphic_map) in Files section
   that could be used for such purpose.

/Pavel


Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk