Re: [boost] uBLAS formal review

From: Fernando Cacciola (fcacciola_at_[hidden])
Date: 2002-06-27 15:47:42

I continue to scan the code...

storage.h
~~~~~~

The macro 'common()'  MUST BE NAMED DIFFERENTLY, perhaps...
UBLAS_MACRO_FUNCTION_WHICH_IS_CALLED_COMMON()
 :-)  (pun intended)

) Why does the default ctors for bounded_array<> et. al. dynamically
allocate an empty array (of size 0)?

) Why does the destructor of bounded_array<> et. al throw bad_alloc() if
data_ has not been allocated? Destructors should not throw exceptions, and
even this one in particular seems highly unnecesary.

) Why do all dynamic allocations via 'new' are followed by 'if (!data_)
throw std::bad_alloc'? "new" already throws bad_alloc if memory is exausted.

) I highly dislike the names 'insert' and 'erase' here. They DO NOT insert
or delete elements, they just set and reset the values of the already
allocated elements. I'd prefer 'set' and 'reset'.

) both arrays could (should?) use valarray<> internally instead of explicit
heap allocated arrays. The difference would be significant because the calls
to std::copy could be replaced by valarray specific and usually optimized
functions.

All this issues seems to be implementation details, except for 'common', and
maybe 'insert' and 'erase', but sometime after inclusion this should be
considered because here there is source for inefficiency, and uBLAS is all
about efficiency, right?

Fernando Cacciola
Sierra s.r.l.
fcacciola_at_[hidden]
www.gosierra.com