From: Douglas Gregor (gregod_at_[hidden])
Date: 2002-01-03 00:05:49
My review of the Multi-Array library follows. I read through the
documentation, tried a few examples, but was unable to give the source code a
careful reading due to time limitations.
Overall, I think that the Multi-Array library should be accepted into Boost.
More detailed comments follow, but my vote for acceptance is not conditional
on anything stated below. I believe that the library is in excellent shape
as-is, and I found the interface to be very intuitive.
General documentation comments:
- I dislike the "almost" conformance to RandomAccessIterator; not because
there is an inherent problem with not having a true reference, but because
"almost" modeling to a concept doesn't really give us any information about
the concepts that are modeled. I'd much prefer to see a separate concept or
set of concepts enumerated that it does fit, and within those concepts the
differences from RandomAccessIterator can be explained. Jeremy's new iterator
categories would be a reasonable set of concepts to use: then the
multi_array::iterator types would model RandomAccessTraversalIterator,
ReadableIterator, and (for non-const) WritableIterator.
- At several points, const_iterator is said to model OutputIterator. This
should be InputIterator.
Main documentation (index.html):
- I dislike the phrase "or fatal as under MSVC." Compiler bugs may be
obnoxious, but we shouldn't be taking shots at vendors within documentation.
MutableMultiArray Concept documentation:
- index_range and extent_range have the exact same documentation string,
which can lead to confusion (it did for me).
- The "Post-condition" column need not exist unless it is going to be used.
- The origin() function does return the address of "a...", unless
all of the indices are zero. It returns the address of "a(a.index_bases())"
- I think that the MutableMultiArray and ConstMultiArray concept documents
should be merged into a single MultiArray concept document, with comments
describing the (relatively few) differences. This is just a matter of
preference, of course.
multi_array class documentation:
- Overall: I would like to see Standardese documentation (i.e.,
- What happens when the dimensions don't agree when the assignment operator
is called? Undefined behavior? Exception thrown?
- The assign(values) form (that assigns to the multi-array from elements in
the sequence [values, values + num_elements())) strikes me as being _very_
dangerous, because it would be very easy to overrun your input buffer. I
would (strongly) suggest removing this form.
- Does assign(first, last) require that std::distance(first, last) equal
num_elements()? I believe that it should, because a partial assignment is not
an assignment, and generally it signifies a user error if one tries to assign
to a multiarray a different number of elements than the multiarray can
- The operator()(indices) and data() member function descriptions make that
same mistake as in the origin() function, mentioned above.
- Why is it that the concept checks are commented out of the source (i.e.,
- There were some problems compiling with Comeau's strict mode, but I will
contain the author directly with those fixes. They aren't important enough to
warrant mailing list traffic.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk