Hi Fredrik, thank you for the reviewing effort! ----- Mensaje original ----- De: Fredrik Blomqvist <fredrik_blomqvist@home.se> Fecha: Martes, Marzo 30, 2004 11:40 pm Asunto: [boost] Re: Formal Review: Indexed Set [...]

* What is your evaluation of the design? I don't feel quite comfortable with the assymetry of index-0 not needing a get<> accessor. I'd prefer forcing the use of get<> everywhere but I guess I can live with it if thats decided. More real-world usage should reveal wether a substantional amount of use-cases naturally benefit from this bias.

Others have also expressed dislike for this feature. Admittedly, it is easier to go the other way around: releasing first without default get<0> and gaining experience to see if it's worth adding on a later release. OTOH, I find the feature useful... I don't know.

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.

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.

* What is your evaluation of the implementation? [...] -I think it would be better if the iterators (index_iterator.hpp) were generated using boost.iterator_facade/adapter instead of the boost.operatorshelper. This would make it easier to adapt and for example directly support the new iterator concepts.

Ummm... will look at it.

-This is more of a "feeling", but could perhaps (parts of?) the 'member'functionality be refactored to use boost::bind/mem_fn code instead?

I don't think so. member<> and the other key extractors are polymorhic, meaning that they accept not only objects of type T, but also reference_wrapper<T>'s, pointers to T and some more. I cannot figure out how to do this with boost::bind.

* 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.

-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.

-Several of the code example in the tutorial section could be slightly reformatted to use less horizontal screen space (see for example the 'Key extraction' part). I'd rather scroll vertically than horizontally! Simply pulling in some of the end of line comments would fix most of it.

Previously reported request, it's in my todo list.

* What is your evaluation of the potential usefulness of the library? [...] * Did you try to use the library? With what compiler? Did you [...] Hope to be able to use it in larger scale shortly.

Please do it and report your experience.

* How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? [...] * Are you knowledgeable about the problem domain? [..] * Do you think the library should be accepted as a Boost library?

Yes, I vote for acceptance.

Well, thank you! I hope you can find this lib of some use in the future. Regards, Joaquín M López Muñoz Telefónica, Investigación y Desarrollo