RE: [boost] Re: Formal Review: Indexed Set
Hi Gary, thanks for reviewing the library! You are welcome. It's always a pleasure to see another cool library.
Have you looked at the performance section? No, will do soon.
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.)
yeah but vector, does a "move"... blech.. its in the details section, do what you want.
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
Namespace: I like boost::container::multi_indexed_sets But I could live without the "::container". I'm not crazy about lifting the namespace into boost. I foresee that boost is going to get cluttered so some organization early will help. "mix" hits my pun loving funny bone, hence it shouldn't be used! Besides "mix_in" has a different meaning and will confuse people. I don't have to type the name of the container very often in comparison to everything else so a clear name is worth a few extra characters. Regular indices: I like "ordered indices" as it says what they are. I hate writing documentation when readinghte code explains everything clearly. Index names: ordered_unique ordered_non_unique They say what they do. Always good for self docuementing code. "set_like" sounds so 90's teenage talk... "whatever.." Nth_index_type & family. Yeah, I'd like to see the _type dropped. If you have to have a unique name, "_t" is ok, but still sucks. Header Organization: The only reason for omitting dependant headers is if they are not "dependent." I.e. that you need the sub header for accessing the container, but the primary header is all that is needed to declare the container. Otherwise include everything that is required. (IMO)
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.
BTW, once some time has passed, get another one. There are plenty of nice ones that need a home. Least ways there are always a heap of cats in the pound. They are never the same but each has been worth it. -Gary-
Powell, Gary <powellg <at> amazon.com> writes:
Have you looked at the performance section? No, will do soon.
There you'll find some performance measures of indexed_set against compositions of several STL containers. Maybe this is what you were after.
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.)
yeah but vector, does a "move"... blech.. its in the details section, do what you want.
sequenced_index_remove is in the detail subnamespace, but sequenced_index::remove is a public memfun of sequenced indices, so it is not only a matter of personal taste. Let me ellaborate. Maybe the docs don't explain it clearly enough (please check the reference) but the idea is that sequenced indices mimic the interface of std::lists, which do have such a remove operation in detriment of erase(value). I don't know why std::list has remove instead of erase (maybe because remove is linear time?) So, regular indices have erase(it) erase(it1,it2) erase(key) and sequenced indices provide erase(it) erase(it1,it2) remove(value) in sync with std::sets and std::lists, respectively. Darren Cook proposed that interfaces be made regular between different indices to ease writing generic code. He proposed it for insert operations, but the same applies for erasing. So I could duplicate some ops in sequenced indices and have the following (an asterisk sigals memfuns currently not provided): regular indices: insert(value) insert(it,value) [hinted insertion] insert(it1,it2) erase(it) erase(it1,it2) erase(key) sequenced indices: *insert(value) [alias of push_back(value)] insert(it,value) [it is not a hint but the insertion position] *insert(it1,it2) [iterated push_back] erase(it) erase(it1,it2) *erase(value) [alias of remove(value)] [other std::list-borrowed ops as they stand now] Have I made myself clear? Does this address your concern? I don't have any strong opinion in favor of / against this, but others in the list may have. That was a long answer, allow me to continue with the rest of your points in a separate post. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
BTW, once some time has passed, get another one. There are plenty of nice ones that need a home. Least ways there are always a heap of cats in the pound. They are never the same but each has been worth it.
Yes, my gf says it's about time to be three again :)
I've been working on my review of indexed set and I've run into a problem. I suspect it is my fault, but I'm having trouble figuring it out. The compiler errors are just not helping.... The basic premise is to setup a bi-directional map using a date and a string as the 2 index types. When I iterate over the first index (a date) everything is fine. But I'm not able to figure out how iterate over the string (second) index. First I used the map_itr as defined below, but that didn't work. I presume that's because the first itr type is for the first index type. But then I haven't been able to figure out the magic to get an iterator type for the second index. I'll send you the compiler errors if you want, but I'm guessing you'll know what I'm doing wrong without this. I'm using gcc 3.3.1 on Mandrake Linux 9.2 and I've already successfully built and run the tests. #include "boost/date_time/gregorian/gregorian.hpp" #include "bimap.hpp" //includes the bidirectional_map template from examples #include <string> #include <iostream> using namespace boost::gregorian; typedef bidirectional_map<date, std::string>::type date_to_string_bimap; typedef date_to_string_bimap::value_type map_vt; typedef date_to_string_bimap::const_iterator map_itr; //works fine for date iteration // why doesn't this work? //typedef nth_iterator_type<date_to_string_bimap,2>::type to_map_itr; int main() { date_to_string_bimap ds_map; date d1(2004,Mar, 27); std::string s1("String 1"); ds_map.insert(map_vt(d1,s1)); //fine here //presumably this needs to be an nth_iterator type? //Not able to iterate over the string side of this... to_map_itr i2 = ds_map.get<to>().begin(); while (i2 != ds_map.get<to>().end()) { std::cout << "string: " << i2->first << " date: " << i2->second << std::endl; i2++; } return 0; };
Namespace: I like boost::container::multi_indexed_sets But I could live without
Powell, Gary <powellg <at> amazon.com> writes: Reply to the 2nd part of your post, here I go. the "::container".
multi_indexed_sets? did you mean indexed_sets or is this a new proposal for the namespace name?
I'm not crazy about lifting the namespace into boost. I foresee that boost is going to get cluttered so some organization early will help.
As I say in the review notes, I'll do whatever people agree on. My only concern with having the namespace as a sub of boost::container is that ids that long as boost::container::multi_index will favor using directives or namespace aliases. Anyway.
"mix" hits my pun loving funny bone, hence it shouldn't be used! Besides "mix_in" has a different meaning and will confuse people. I don't have to type the name of the container very often in comparison to everything else so a clear name is worth a few extra characters.
Yeah, I think you're right. mix dropped (seemed like I was the only one that liked it). We are left then with indexed_set, multicontainer, multi_container.
Regular indices: I like "ordered indices" as it says what they are. I hate writing documentation when readinghte code explains everything clearly.
Index names: ordered_unique ordered_non_unique They say what they do. Always good for self docuementing code.
"set_like" sounds so 90's teenage talk... "whatever.."
I like them too. Future hashed indices can be called unordered_unique and unordered_non_unique, in heavenly harmony with upcoming std hashed sets.
Nth_index_type & family. Yeah, I'd like to see the _type dropped. If you have to have a unique name, "_t" is ok, but still sucks.
The only places where dropping "_type" causes problems is iterator_type<> and const_iterator_type<>. But wait, I think I have something that may please everybody: template<int N> struct nth_index; template<typename Tag> struct index; template<int N> struct nth_index_iterator; template<int N> struct nth_index_const_iterator; template<typename Tag> struct index_iterator; template<typename Tag> struct index_const_iterator; Comments?
Header Organization: The only reason for omitting dependant headers is if they are not "dependent." I.e. that you need the sub header for accessing the container, but the primary header is all that is needed to declare the container. Otherwise include everything that is required. (IMO)
This seems to lead to a finer-grained separation. OK with me, monolithic headers do not seem to be on vogue at Boost. Joaquín M López Muñoz Telefónica, Investigación y Desarrollo
participants (3)
-
Jeff Garland -
Joaquin M Lopez Munoz -
Powell, Gary