"Thorsten Ottosen" <nesotto@cs.auc.dk> wrote in message news:cjvavc$hot$1@sea.gmane.org...
Hi Jonathan,
| 4. Since performance is one of the main design goals, I'm suprised that | attempting to insert a null pointer causes an exception rather than an insertion | failure. The same goes for invoking the functions for back, front, pop_back and | pop_front with an empty container. By coincidence, this morning I was | experimenting with some code that invokes a function pointer with a pointer as | argument, like so | | f_(p_) | | in an inner loop. I tried replacing the above code with | | if (p_) | f_(p_); | else | throw std::runtime_error(...); | | I found an enormous slowdown, which should be no surprise.
yeah, I dunno, this might depend heavily on the compiler. what compiler do you use?
Good point. I was using VC7.1. I ran the same test with como4.3.3/VC7.1 Intel 8.0 for windows and GCC 3.4.3 (Cygwin) and didn't see a significant difference. But the fact that it makes a difference for some compilers, esp. a widely used one, should be significant.
At least for back()/front() you can avoid a check by *ptr_begin()/*--ptr_end(). For vector/deque you can unchecked random access too via operator[]().
It is is true that performance was a design goal, but so was safety. The domain of pointers allowed me to do different trade-offs than with standard containers. For example, does it really matter if container.push_back( new T ) throw if 0 was inserted? IMO no, because the heap operation will dwarf any other operation by several magnitudes. The same considerations would be true for eg. pop_back() when calling the detructor on the object since we need to remove the object from the heap.
Also good point. I think it doesn't matter for popping, for the reason you mentioned. For inserting elements, it won't matter most of the time, but sometimes you might be transfering ownership of already constucted objects. I think it's a bit odd to require someone who wants uncheked access to the final element to use *--ptr_end().
| 6. To answer possible review issue 13, I'd say you should parameterize the | provided clone managers by allocator type, defaulting to std::allocator< | [unspecified] >.
why unspecified instead of T* ?
Right. I was looking around for a T, but I didn't see one since heap_clone_manager and view_clone_manager are non-templates. Obviously you just use the T from the container template declaration.
| 7. I don't understand review issue 10. I don't see the erase()/insert() | functions that return pair<iterator, bool>.
in set/map and ptr_set/ptr_map these functions returns this pair to say "we did/did not insert the element".
But I can't find these members in the documentation. Best Regards, Jonathan