chessweb
June 23rd, 2002, 04:45 AM
Hi,
I have to allocate memory for a 2D ragged array and I thought I had it figured out. But then I discovered that the following code causes a memory leak:
// Allocate memory for a 2D-array with varying row length
int no_of_rows = 10;
int* no_of_cols = new int[no_of_rows];
for(int i=0; i<no_of_rows; i++)
infilestream >> no_of_cols[i];
CMyClass** myO = new CMyClass*[no_of_rows];
for(i=0; i<no_of_rows; i++)
myO[i] = new CMyClass[no_of_cols[i]];
... code using myO[i][j] ...
// Deallocate the memory
for(i=0; i<no_of_rows; i++)
delete [] myO[i];
delete [] myO;
delete [] no_of_cols;
The problem is with the deallocation of the array. But I don't know how to do it differently. Any hint on a solution would be
greatly appreciated.
Regards, Ralf
Paul McKenzie
June 23rd, 2002, 11:38 AM
You could have used std::vector and saved yourself from calling new[] and delete[]. I don't see anything glaringly wrong with what you did (except for one spot where your code wouldn't have compiled under an ANSI C++ conforming compiler), unless there is a bunch of code you left out.
Maybe one of the "no_of_cols" values is not what you think it is (a negative number perhaps?). Did "no_of_rows" change value in between the time you allocated and the time you deleted?
I would just create it, check that the numbers being read in are valid and deallocate it, without doing any other operations, and see if there really is a memory leak. What did you use to come to the conclusion that there is a memory leak?
Anyway, here is how to do what you want using std::vector<>
#include <vector>
// Assume that CMyClass has been defined
// plus other necessary include's have been done
// Define a type. Make sure there is a space between the last
// two "> >"!!
typedef std::vector<std::vector<CMyArray> > CMyClass2D;
/* Or to illustrate with more granularity...
typedef std::vector<CMyClass> CMyClassArray;
typedef std::vector<CMyClassArray> CMyClass2D;
*/
int main()
{
// Declare a vector of 10 integers
int no_of_rows = 10;
std::vector<int> no_of_cols(no_of_rows);
// I'll explain why this is placed outside later
int i;
for (i = 0; i < no_of_rows, ++i)
infilestream >> no_of_cols[i];
//...
int rows_in_table = no_of_cols.size();
// myO is the 2d array.
// "Allocate" the rows
CMyClass2D myO( rows_in_table );
// Resize each row to give it the raggedness.
for (i=0; i < rows_in_table; ++i)
myO[i].resize(no_of_cols[i]);
//...
// Get the number of elements in row 0 and row 1
int elements0 = myO[0].size();
int elements1 = myO[1].size();
//...
// Now you can use myO[x][y], just like an ordinary 2-d array
my{0][0] = 14; // Assuming that row 0 has at least 1 column.
}
Note, the code has not been compiled (of course), so be wary of typos. But the point is hopefully illustrated. There is no need for new and delete. The vector<> does this work for you. I also showed how to get the size of the vector by calling the vector::size() member function. This illustrates that you don't have to lug around variables that give you the number of rows, or how many elements are in each row.
Now to explain the error in your code regarding ANSI C++.
For a conforming compiler, variables defined in the for( ) loop have scope only within the for() loop. In your code, you have
for(int i = 0...)
but later on, you have
for( i = 0...);
Since "i" has gone out of scope, this is an error. However VC++ lets you get away with it, in fear that conforming to ANSI C++'s for() definition would break a lot of code. That's why I moved the definition of "i" outside the loop, just in case you decide to try this on another compiler. The correct ANSI C++ way would have been to say, again "for (int i=...)".
Regards,
Paul McKenzie