Re: [boost] uBLAS formal review

From: Joerg Walter (jhr.walter_at_[hidden])
Date: 2002-06-27 17:37:41

----- Original Message -----
From: "Fernando Cacciola" <fcacciola_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Thursday, June 27, 2002 10:47 PM
Subject: Re: [boost] uBLAS formal review

> 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)

Ouch, yes.

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

Why shouldn't they?

> ) 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.

Bad behaviour due to MSVC education.

> ) 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.

Not under MSVC.

> ) 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'.

But they insert and erase in the sparse case and they are used generically
(without partial specialization again due to MSVC).

> ) 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.

Could, but shouldn't.

> 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?

Regards

Joerg