Boost logo

Boost :

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[0][0]...[0]", 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.

Code comments:
  - Why is it that the concept checks are commented out of the source (i.e.,
in multi_array.hpp)?
  - 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, gregod at, cpdaniel at, john at