From: jdm on
I'm trying to rewrite some code that used arrays and pointers like
this:

void calculate_table(size_t rows, size_t columns, int *raw_data, int
**table)
{
//initialise the elements of table to zero.
//then save the results of calculations on the elements of raw_data
in it
}

so that it will use the standard library instead. Sometimes there have
been situations where I've had to leave the original function intact,
and so to avoid duplication have ended up writing something like:

void calculate_table(size_t rows, size_t columns, vector <int>&
raw_data, vector< vector <int> >& table)
{
int **array_table = new int*[rows];
for (size_t s = 0; s <rows; s++)
{
array_table[s] = &table[s][0];
}

calculate_table(rows, columns, &raw_data[0], array_table);
delete[] array_table;
}

(exploiting the fact that the elements of a vector have to be stored
contiguously, as per the C++ standard.)

Is there a more efficient way of doing it than that for loop? Some of
these functions will have to be called a very large number of times
during some simulated annealing, and I may soon have to do something
like the above with some code involving three-dimensional arrays.

Thanks,

James McLaughlin.

--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Ulrich Eckhardt on
jdm wrote:
> I'm trying to rewrite some code that used arrays and pointers like
> this:
>
> void calculate_table(size_t rows, size_t columns, int *raw_data, int
> **table)
> {
> //initialise the elements of table to zero.
> //then save the results of calculations on the elements of raw_data
> in it
> }

Two aspects to this:
1. The "int* raw_data" resembles a "vector<int>& raw_data", though I guess
you want to pass a reference-to-const instead.
2. The rows, columns and table (btw: why are these separate in the parameter
list?) for a 2-D array. A vector of vectors doesn't cut it quite, as there
every vector can have its own size. I would rather a matrix class, e.g.
found in Boost. If you don't want to use that, write you own or just pass
in a vector of integers along with the number of rows and columns. You can
then calculate the index of element N, M with I = N * columns + rows. I
would have preferred a similar method in C, too, because it saves multiple
allocations, checks, deallocations etc.

> so that it will use the standard library instead. Sometimes there have
> been situations where I've had to leave the original function intact,
> and so to avoid duplication have ended up writing something like:
>
> void calculate_table(size_t rows, size_t columns, vector <int>&
> raw_data, vector< vector <int> >& table)
> {
> int **array_table = new int*[rows];
> for (size_t s = 0; s <rows; s++)
> {
> array_table[s] = &table[s][0];
> }
>
> calculate_table(rows, columns, &raw_data[0], array_table);
> delete[] array_table;
> }
>
> (exploiting the fact that the elements of a vector have to be stored
> contiguously, as per the C++ standard.)

Argh, nooo!!!1! If you allocate dynamically, use a vector! In this case,
where you needed an array of pointers, use a vector<int*>.

> Is there a more efficient way of doing it than that for loop? Some of
> these functions will have to be called a very large number of times
> during some simulated annealing, and I may soon have to do something
> like the above with some code involving three-dimensional arrays.

Allocating, copying, deallocating are expensive. Avoid them if possible. If
there are some low-level functions that you can't change, design your
datatypes for them. You can still wrap them in a class so that you get
assignment operators, copying, proper resource management etc.

Uli

--
Sator Laser GmbH
Geschäftsführer: Thorsten Föcking, Amtsgericht Hamburg HR B62 932


[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Nick Hounsome on
On 18 May, 22:52, jdm <james.d.mclaugh...(a)gmail.com> wrote:
> I'm trying to rewrite some code that used arrays and pointers like
> this:
>
> void calculate_table(size_t rows, size_t columns, int *raw_data, int
> **table)
> {
> //initialise the elements of table to zero.
> //then save the results of calculations on the elements of raw_data
> in it
>
> }
>
> so that it will use the standard library instead. Sometimes there have
> been situations where I've had to leave the original function intact,
> and so to avoid duplication have ended up writing something like:
>
> void calculate_table(size_t rows, size_t columns, vector <int>&
> raw_data, vector< vector <int> >& table)
> {
> int **array_table = new int*[rows];
> for (size_t s = 0; s <rows; s++)
> {
> array_table[s] = &table[s][0];
> }
>
> calculate_table(rows, columns, &raw_data[0], array_table);
> delete[] array_table;
>
> }
>
> (exploiting the fact that the elements of a vector have to be stored
> contiguously, as per the C++ standard.)
>
> Is there a more efficient way of doing it than that for loop? Some of
> these functions will have to be called a very large number of times
> during some simulated annealing, and I may soon have to do something
> like the above with some code involving three-dimensional arrays.

This is a terrible idea.
Best to create a matrix class and implement metjods on it in terms of
the C functions.
Your tables appear to be ractangular rather than ragged (as is implied
by Vector<vector>) so you can have something like (I'm guessing your
calculate_table here):

class Table
{
int** m_table;
int m_rows;
int m_cols;

void calculate(const vector<int>& raw_data)
{
::calculate_table(m_rows,m_cols,const_cast<int*>(&raw_data[0]),m_table);
}
}

Another incremental approach is to change calculate_table to a
template function so that what is now int** can be called with Table&
and add a forwarding overload to avoid having to explicitly pass the
rows and cols params.

The bottom line is that nobody will thank you for breaking working
code so it is usualy safer to wrap rather than rewrite.


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]