JOAQUIN LOPEZ MU?Z wrote:
Hi Fredrik, thank you for the reviewing effort!
----- Mensaje original ----- De: Fredrik Blomqvist <fredrik_blomqvist--at--home.se> Fecha: Martes, Marzo 30, 2004 11:40 pm Asunto: [boost] Re: Formal Review: Indexed Set
[snip]
I also second the requests for lifting an implementation of a bidirectional_map/set to official status. But I don't expect nor demand this to be added before acceptance, I'm fine with it being a reasonable prioritized roadmap feature.
Count on it. I'm emotionally attached to bimap, as this container was that started it all.
Great! (I did read your fine Codeproject article about it also)
Serialization support is also high on my wish-list (but with same remarks as above).
It is already in the todo list. I'll try to reserve some time to enter into the serialization review and use this case as a testbed. Nice to hear. Indexed_set + serialization will be powerful stuff indeed!
[Snip]
* What is your evaluation of the documentation? [...] Some nitpicks: -In the example at the end of the tutorials section ('Projection of iterators') I think it would be clearer if it would use equal_range instead.
You cannot! I was bitten by this at my first try to write the snippet. Suppose we do an equal_range() on "sister" and get the range [it0,it1): it0 will point to a sister and *it1 will point one-past the last occurrence of "sister". Imagine now that we are prepending "younger" instead of "older" and that *it1 is "zoe". Then
while(it0!=it1;++it0) { // get it2 from it0 by means of project() tc.insert(it2,"younger"); }
kah-boom! Now, *as seen by index #1*, "younger" gets between the last occurrence of "sister" and "zoe" (alphabetical order). So it1 will be pointing off beyond the "sister" range (in fact, an infinite recursion ensues.) I could have fixed this by considering (--it1) instead, but I thought the way I settled on was clearer for explanatory purposes.
Doh! Seems like I only looked at the iteration... totally neglected the fact that you were modifying the set! sorry...
-I find the coding style to be slightly too compact. No space after ',' and '&' etc makes it more difficult to quickly browse. Why not just follow the semi-official boost coding guidelines? (currently found in the file area)
No particular reason for the style, I've been coding like this for 10 yrs. Other reviewer requested that lines did not exceed 80 chars, which I've just done. But inserting the missing blanks is a pain in the ass, and I'll probably start coding again like before as soon as I forget about it.
Ah, I didn't really mean that you needed to change _all_ the main code (just a hint ;-). My point was mostly that the code snippets in the _documentation_ should be changed. Still slightly tedious I guess.. (hmm, a boost::pretty-printer based on spirit tech would be a cool little util :) ) Regards // Fredrik Blomqvist