From: Jens Maurer (Jens.Maurer_at_[hidden])
Date: 2000-06-26 16:33:30
Ok, here are my review comments before the time runs out.
In general, I think the class is well thought-out and simple.
The examples are ok.
I vote for inclusion into boost.
I'm not sure about the name, but for me "c_array" rings more
bells than simply "array". "block" is definitely out of the
game IMHO. Oh, and "carray" is too munged for my feeling.
But anyway, I don't really care much.
Can we stick more closely to the boost look & feel for that page
and have (at least) the boost GIF on top? Maybe even the navigation
"many feedback": Is it really countable? I thought it was
"Class array fulfills most but not all of the requirements of
reversible containers (see Section 23.1, [lib.container.requirements]
of the C++ Standard). The class is not an STL container because ..."
This tells you that it isn't quite a reversible container and then
it explains why it isn't a container. Are there any other things
required for a reversible container (as opposed to a simple container)
which are missing from the library? [No, there aren't: please say so.]
Instead of "Home Page", I would appreciate the author's name
clearly visible on the web page.
Comment after #include´s: // BUG-FIX for compilers that doesn't
should say "... don't support" (plural!)
The typedefs in namespace std are non-conforming (because they add to
namespace std in an invalid manner) and they are not guarded by
#ifdef BROKEN_COMPILER (whence this would be tolerable, see
Additionally, multiple such typedefs (for example, if another
heaeder file had the same idea) violate the one definition rule.
Summary: #include <boost/config.hpp> and forget about the issue.
The rangecheck() member function should be declared "const", otherwise
the call from the const at() member function looks invalid.
As mentioned elsewhere, make size(), empty() and max_size() static.
Do we want the "using std::swap;" trick so that Koenig lookup finds
more global element swap() functions?
"// comparisions": My dictionary says the second "i" is superfluous.
The example should reference "boost/array.hpp" and not simply
"array.hpp". I don't care for <...> or "...".
We should not use std::endl too often, because it calls flush(),
which is usually a performance bottleneck. Nearly all output
statements can be coalesced to one big expression. After all,
it's an example program, so it should show good manners :-)
I don't like "using namespace", and particularly not in
example programs. I favour putting "std::" where it is
PRINT_ELEMENTS() uses all caps, which should be reserved for
Has another print_elements() implementation.
Incorporate array3.cpp´s print_elements() into array2.cpp
and do away with print.hpp, since it's used only in array2.cpp
Enough for now.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk