[boost] [bimap][review result] Bimap Review Summary

From: Ion Gaztañaga (igaztanaga_at_[hidden])
Date: 2007-03-11 10:33:27

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