Hi to all, First of all, sorry for being late with this review summary for Bimap. As I mentioned in my review result post, reviews have been very positive, but there are still a few design decisions that weren't fully ironed in the review threads. Let's enumerate the reviews: * * * * * * * * * * * * * * * * * * * * * * * * * * * [start of summary] Alisdair Meredith ----------------- Alisdair Meredith did a positive review and kindly requested support for Borland compilers. This would require changes in Boost.MultiIndex (on which Bimap is based). Fernando Cacciola ----------------- Fernando Cacciola suggested improving the introduction "What is a bimap" with a picture explaining the nature of a bimap. Matias has already created a new picture based on Fernando's suggestions. The only point Matias does not agree is that the "above" view is now written in the bottom of the picture. Fernando did some suggestions to clarify some paragraphs in the documentation (e.g the Tutorial) and Matias agreed to modify some of them. The picture of the "standard mapping framework" section was also mentioned as a possible point of improvement. Jeff Garland ------------ Jeff noted that the namespace "bimap" of the documentation should be boost::bimap (- note of the review manager -: the namespace issue is discussed later in this summary) and suggested a top level boost/bimap.hpp header. He also suggested changing the example from the tutorial to use "bimap::value_type" instead of "bimap::relation". Matias considered that the one minute tutorial should only contain the sides views and that set_of_relations should be introduced later. Jeff commented that const iterators should be used in the "A simple example" code and considered essential adding an example showing the behavior when inserting a key that is already in the container. He considered that the tutorial should have more examples, and in general, more documentation explaining the many features bimap offers. std::map compatibility: Jeff proposed making bimap compatible with std::map, but Matias replied that the left/right views are the right answer for std::map compatibility and that he wanted to maintain bimap as a new view of the container (a set of relations). (- note of the review manager -: this issue is discussed in another later) John Maddock ------------ Proposed a longer description of what a bimap is and considered that "tagging" description should be improved and proposed changing the title of the section. Since the complexity notation is quite different from the usual one, John proposed linking complexity notation with the explanation of terms. John detected some errors while compiling the examples related to the namespace name issue. Thorsten Ottosen ---------------- (- Note of the review manager -: This thread was the longest thread of the review: operator[](),at() and first-second/left-right, were the key topics. The left-right issue will be discussed later on this summary post.) Thorsten considered operator[] complicated. Matias explained that that current implementation is the most conservative one. Thorsten would maintain only the non-mutable versions or just remove the function. Matias agreed to remove the non-mutable version but still had doubts. Thorsten proposed adding at(). Matias agreed to add only the non-mutable version and commented that the mutable version should only be available if the other view is not associative to avoid breaking container invariants. Several member additions were discussed: --> "replace" overloads: replace(it, key, value), replace(it, key) and replace(it, value) --> insert( const key&, const key2& ). This would need Boost.Multiindex changes. Joaquin commented that this would need a bit of work and that there were some allocator interface limitations that he would need to avoid. Placement new was suggested. The issue remains open. --> Insertions from convertible values. The goal is to reduce the number of called copy-constructors. --> Construction from keys without constructing a relation lead to a discussion about how Boost.MI could offer the needed infrastructure to achieve a delayed construction facility. Joaquin put this issue in his "to study" list. Apart from this, Thorsten suggested improvements to the documentation text in several sections. Tony (I couldn't get the surname, sorry) ---- "One Minute Tutorial" needs two additional lines: #include <boost/bimap/bimap.hpp> using namespace bimap; Tony considered "relation" too generic and proposed "couple"/"duo"/"bipair" alternatives. Matias replied that he preferred staying with "relation" first-second and left-right thread ---------------------------------- Matias started a new thread explaining the reasons behind the map<X,Y> interface for side views and set<relation> for the "above" view. John Maddock suggested a ".relation" member to access to the set<relation> facilities. Thorsten agreed. Matias considered that there are reasons to maintain set<relation> functions as bimap members (for example "size()"). This issue remains open with the following key points: --> Should bimap offer "relation" views? --> In that case, should this view be accessed using a ".relation" (or other name) member or should we consider bimap as the view? --> Should "relation" (or another name) contain left-right or first-second members? I hope this issues will be solved soon with mailing list discussions. new name for the container -------------------------- According to Boost rules, the namespace should be called boost::bimaps. Thorsten proposed injecting boost::bimaps::bimap into the boost namespace. A <boost/bimap.hpp> header was also considered useful. [end of summary] * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * I've tried to collect the most important points of the review process, but obviously I might have missed some points or misunderstood some positions/conclusions. If that's the case, please correct me! Regards, Ion