
Looks like I didn't make it in time. Ion just announced the result, and I congratulate with Matias for the result of the review. I'll add here my (short) review of the library anyways. First of all, I think that the library should be accepted, but that doesn't matter anyway now :) * What is your evaluation of the design? The design looks sound. Probably the interface is a bit complex for being a simplified wrapper around MI, but on the other hand, Bimap is certainly not a simple wrapper. Still the most common use is bimap<A, B>, and Boost.Bimap makes the simple case simple. I do not buy much the reason for preferring left/right to first/second. At least in an std::pair, their name does not reflect the importance in the sequence (as it might be in an std::map), but just their position in the signature. For the same reason, bmap could use these tags to name the left and right view. The tutorial states: "Be aware that a bidirectional map is only signature-compatible with standard containers. Some functions may give different results, such as in the case of inserting a pair into the left map where the second value conflicts with a stored relation in the container. The functions may be slower in a bimap because of the duplicated constraints. It is strongly recommended that you read The full tutorial if you intend to use a bimap in a serious project." Wouldn't be better to make functions with different behaviour from standard containers *not* signature compatible? I've been bitten at times with function signature compatibility but behaviour incompatibility in standard containers. The lambda support names the placeholder for the left, right, first, second and key with a underscore in the beginning (i.e. "_first"). While I think this is done to mimic the "_1" notation, I think that here the underscore is not really necessary. Probably just "first" would just be better. Also shouldn't support for pair be directly inside boost::lambda? (speaking of it, can we also have built in support for tuples in lambda? Pleeease). * What is your evaluation of the implementation? Didn't study it enough to make a comment. * What is your evaluation of the documentation? Very Good. It is very clean and I like the graphic aspect, while maintaining a Boost look and feel. The pictures used to explain relations help a lot. I also like the color scheme. The index page has a very nice image based menu. While it looks great, it makes it harder to navigate it using the "Find as you type" functionality of Firefox. I use this functionality a lot when navigating documentation. A test based duplicate could be exposed in the page. The one minute tutorial explains very well the the basic bimap<A, B> interface. I've found parts of the extended tutorial a bit non intuitive. In particular the part about "The set of relations type". I had to read it a couple of times, and I do not feel that I still get it completely. But that Is probably just me. The documentation assumes that this is in effect. using namespace bimap; As many have stated before in reviews of other libraries, I think that something like: namespace bm = boost::bimap; would be more appropriate. I think this is not just a matter of style. When reading documentations, I often find myself looking up symbols to see in what namespace are supposed to be (is this symbol from bimap, was it part of another library, is it a local typedef?). Tagging: "member_at::{side}" Here "{side}" is a placeholder for "left/right". But "{" and "}" have meaning in c++, and it took me a while not to parse it as valid code :( . This placeholder syntax is used in other places in the documentation. Probably some other typographic convention should be used. The reference section is impressive, with detailed explanation for every member function. Great job. I liked a lot all the extra sections. The "external dependencies" is welcome. The implementation detail parts provides a very interesting view on some of the rationale for the library. I liked the snippets of conversation with the mentor. * What is your evaluation of the potential usefulness of the library? Extremely useful. While the simple case of two synchronized maps is not hard to do, it is error prone, and often quick ad-hoc implementations do not have the full generality of boost bimap. Going full MultiIndex is an overkill for simple projects (the interface is definitely more complex). The *_of set types add value to the library but probably are going to be less used. * Did you try to use the library? With what compiler? Did you have any problems? No. * How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? I did study the documentation but didn't try the library. * Are you knowledgeable about the problem domain? As many, I had the need to maintain two synchronized std::map to do look up from both sides. I've also used boost.MI. I'm definitely not an expert on MI internals. Ok, that's it. But before finishing, I have thank again Matias for the help in using boost.build and quickboost during the pas SoC. gpd