Tuple review

From: Ed Brey (edbrey_at_[hidden])
Date: 2001-06-18 09:14:28

After a careful consideration of the interface and reading of the
documentation and a light reading of the code, I think that Boost.Tuple
should be included into boost.  Following are comments on the library:

Design:

The nested namespace is quite convenient.  However the namespace name
"tuples" should be "tuple".  (Better reflects its usage at a given point
in a program; it's similar to giving an enum a singular name. Also
provides better correspondence to the header file name.)

Similarly, the "details" directory should be "detail".

A 0-tuple is not defined.  Would such be useful for generic programming?

The ref and cref are quite nice.

tie now exists in both boost and boost::tuple namespaces.  It make sense
to me to deprecate the one in utility.hpp.  Is that the plan?  The
really nice "ignore" functionality which differentiates tuple::tie adds
more weight to the issue.

set_left and set_right seem likely to cause problems with right-to-left
(and top-to-bottom?) languages.  set_open and set_close would probably
be better.

The tier concept could use a new name.  It took me a while to realize
you weren't (mis)using the word that meant "One of a series of rows
placed one above another."  Unfortunately, I can't think of a good
alternative.  If I can, I'll let you know; otherwise, it's this issue
isn't a big deal.

nil is undocumented.  It should probably be either documented or moved
into the detail namespace.  Is the nil/null being proposed as due to the
function library discussion compatible with this?  Assuming yes, it
would seem to keep things minimal to share the single class once it is
complete.  As far as naming goes, null is standard C/C++ terminology for
this concept of referencing nothing when one might otherwise be
referencing something, and it would be good to use the existing name for
this concept.  This is especially true if a C++ 0x might have a null
keyword that could be leveraged by simply deleting the null class.  If a
null keyword would not be drop-in replacement, then it makes sense to
call the class something besides null (e.g. nil) to avoid a future
conflict.

tuple_length likewise is both undocumented and not in the detail
namespace.

Documentation:

"Using the library":

It is unclear whether tuple.cpp is needed for tuples in general, or only
if tuple input and output operators are used.

"Constructing tuples":

The depth of this section is excellent.  Unfortunately, this makes it
quite heavy for novices.  It would be useful to rearrange the documents
putting all the key points a novice needs to get started (e.g. getting
elements) towards the top.

"Note that an argument can only be left unspecified, if it has a default
initialization." is unclear.  Perhaps you mean "If no initial value for
an element is provided, it is default initialized (and hence must be
default initializable)."

'tuple<X,X,X>(string("Jaba"), string("Daba"), string("Duu"))' would be
stronger example as 'tuple<X,X,X>("Jaba", "Daba", "Duu")' (assuming
that's OK).

"tuple<const double&>(d+3.14)    // ok, but potentially dangerous: //
the element may become a dangling reference" could be made stronger.
The element will become a dangling reference.  The only question is
whether the dangling reference will be a accessed.

"Relational perators":

Section title is missing its "O".

"Note that an attempt to compare two tuples of different lengths results
in a compile time error.": It would be worth stating here whether tuples
of different types (but same length) can be compared. (Even though the
example makes this clear, it makes reading easier not having to wait to
get down that far.)

"Streaming":

"cout << set_left('[') << set_right(']' << set_delimiter(',') << a;" is
missing a close parenthesis after ']'.

"Note that extracting tuples with std::string or C-style string elements
most likely does not work": It would be more accurate to say does not
generally work.  For applications where the string contents are
constrained, it can work just fine.  It's not a matter of probability.

It is unclear how long the manipulators stay in affect.  I'm not sure if
this needs to be clarified or if there is a iostream standard that I'm
not remembering off hand.

Code:

"inline" inside structure definition is superfluous.

tuple_io should not include iostream, but rather istream and ostream.