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