RE: [boost] Re: Formal Review: Indexed Set
I have been reading the code, and am working through the docs. I compiled and ran the examples with gcc 3.2. All compiled and ran as advertised. I did try with gcc 2.95.3 but it fails :< I have occasionally needed a container just like this one. So I am glad to see such a nice one being offered to boost. I made an earlier comment about operator== et.al. I don't think its worth holding up this change in attempt to get all the std::containers and boost containers to conform. I did complain about this with circular buffer but I've deleted the response. If no one else complains about this, I'll write a paper for the std committee for presentation in Redmond in Oct 2004. I don't see the harm in fixing containers as they are proposed to boost though. When we first wrote the VTL library we did this with no ill effects. I have not run any timing tests, of using this container vs using two (N) maps with links or whatever with pointers to the same data. A small nit, sequenced_index::remove and sequenced_index_remove should be called "erase". Its a small nit because they live in "details". But really they do erase. I like having index_fwd.hpp I wish all the std::containers had such a file. Also you should look at Dave Abraham's and my paper on Move semantics/copy construction vs Andrei's MOJO. IMO Its worth implementing. :> BTW, I also really like the example that uses boost::lambda to help set up the conditionals! And a plug for my library in no way influences my vote.. no sir yee, I'm totally impartial here... I vote to accept unconditionally. Yours, -Gary- PS Sorry to hear about your cat. I had one that only made it to 3 yrs, and that was hard to take. But cats gotta be cats.
Hi Gary, thanks for reviewing the library! Powell, Gary <powellg <at> amazon.com> writes: [...]
I made an earlier comment about operator== et.al.[...] I don't see the harm in fixing containers as they are proposed to boost though. When we first wrote the VTL library we did this with no ill effects.
I don't see any problem either. I'll implement it and will commit if no weird things show up.
I have not run any timing tests, of using this container vs using two (N) maps with links or whatever with pointers to the same data.
Have you looked at the performance section?
A small nit, sequenced_index::remove and sequenced_index_remove should be
called "erase". Its a small
nit because they live in "details". But really they do erase.
Umm... Sequenced indices are modelled after std::list, which contains both erase and remove operations, with different semantics (remove meaning erasing *all* elements with a given value.)
Also you should look at Dave Abraham's and my paper on Move semantics/copy construction vs Andrei's MOJO. IMO Its worth implementing. :>
Point taken. [...]
I vote to accept unconditionally.
Sweet! Thank you, I knew the lambda hook would work :) If you still have time to devote to the library, please please take a look at the review notes on naming and header organization and cast your opinions: http://groups.yahoo.com/group/boost/files/indexed_set_review_notes.html I'm less than satisfied with some of the current naming decisions, and now it's a good time to improve them. I'm planning on posting a naming scheme candidate by the end of the review, trying to gather reviewers' opinions. Best, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo.
Yours, -Gary-
PS Sorry to hear about your cat. I had one that only made it to 3 yrs, and
that was hard to take. But cats gotta be cats. Thanks. First time I had an animal companion in decades, so I wasn't prepared for this sense of loss.
participants (2)
-
Joaquin M Lopez Munoz -
Powell, Gary