How to delete 2D Array? Thanks!

[zelta]

Hi guys! I've absolutely no idea why my delete codes inside the destructor won't be able to functionally well. I hope u guys can help me for this.

Thank you so much!

Please refer to last post for updated program!Thanks!

[Duthomhas]

What do you mean "it won't work"?

Is your application crashing?


You need to have a copy constructor that duplicates the data. Imagine:

object A: constructed --> allocates memory (via new[]for the 2D array

object B: constructed as a copy of A --> no allocation of a 2D array, instead it shares the memory for the 2D array with object A

object B: destroyed --> the 2D array is freed (via delete[])

object A: thinks its 2D array is still A-OK even though object B (which no longer exists) deleted the 2D array (so now it no longer exists either)


Whenever your objects maintain pointers to dynamically-allocated data, they must be careful to prevent these kinds of problems. In your case, I would just make a deep copy -- that is, duplicate the 2D array -- whenever you copy the Array2D object.

class Array2D
{
    public:
        ...
        Array2D( const Array2D& );               // copy constructor
        Array2D& operator = ( const Array2D& );  // copy assignment operator
    ...
};

/*
  copy constructor
*/
Array2D::Array2D( const Array2D& original )
{
    // Copy the original's attributes (number of rows and columns)
    row = original.row;
    col = original.col;

    // Allocate new space for our copy of the original data
    p   = new int* [row];
    for (int i=0; i < row; i++)
        p[i] = new int [col];

    // Copy the original data into our data
    for (int i = 0; i < row; i++)
    for (int j = 0; j < col; j++)
        p[i][j] = original.p[i][j];
}

/*
  copy assignment operator
*/
Array2D& Array2D::operator = ( const Array2D& original )
{
    // same stuff as above, plus the following:
    return *this;
}

These two methods (copy constructor and copy assignment operator) are part of "the Rule of Three" when programming in C++:
http://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29

You can make it easier on yourself by having a private method (like copy( const Array3D& ); or somesuch) and then calling that method from your copy ctor and copy assign op, which would then look like:

Array2D::Array2D( const Array2D& original )
{
    copy( original );
}

Array2D& Array2D::operator = ( const Array2D& original )
{
    copy( original );
    return *this;
}

Hope this helps.

[zelta]

@Duoas, I really appreciate your effort. Thanks very much!.

Actually I have the 2 functions inside my class. However, the addArray works well but the multiplyArray is failed due to the destructor.

Please refer to last post for updated program!Thanks!
}	  

[Duthomhas]

I can't see how that function could possibly fail except for the reasons I've already stated.

If you can, post your whole code online somewhere where we can peruse it, like pastebin or somesuch.

[zelta]

If you try to execute the below program, you will obtain some garbage values, however, you will obtain correct result if you does not include anything inside the destructor. Thanks!

'#include <iostream>
#include <cstdlib>
#include <ctime>

using namespace std;

class Array2D
{
	public: 
			Array2D();
			Array2D(int, int);
			~Array2D();
			Array2D (const Array2D&);
			

			void addMatrix (const Array2D&);

			Array2D multiplyMatrix (const Array2D&);
	
			void printMatrix ();
	
	private:
			int row;
			int col;
			int **m;
			void initialize ();
			void generateArray2D ();
			
};



static int size=0;
Array2D::Array2D()
{

}


Array2D::Array2D(int rows, int cols)
{
	this -> row = rows;
	this -> col = cols;
	initialize();
	generateArray2D ();
}


Array2D::~Array2D()
{

	for (int i = 0; i < row; i++)
  	{
  		delete[]m[i];
  	}
	delete[]m;

}


Array2D::Array2D(const Array2D& mx)
{
	this -> row = mx.row;
	this -> col = mx.col;
	
  	initialize();
	
	for (int i = 0; i < row; i++)
		for (int j = 0; j < col; j++)
			m[i][j] = mx.m[i][j];
}


void Array2D::initialize()
{
	m = new int* [row];
	
	for(int i=0; i< row; i++)
		m[i] = new int[col];
}


void Array2D::generateArray2D ()
{
	for (int i = 0; i < row; i++)
		for (int j = 0; j < col; j++)
		{
			m[i][j] = rand () % 10;
		}
}




void Array2D::addMatrix (const Array2D& mx)
{
	for (int i = 0; i < row; i++)	 
		for (int j=0; j < col; j++)
			m[i][j] += mx.m[i][j];	 	       
}



Array2D Array2D::multiplyMatrix (const Array2D& mx)
{
	int rowAB = row;
	int colAB = mx.col;
	
	Array2D ab(rowAB, colAB);
	
	for(int i=0; i< row; i++)
	{

		for(int j=0; j< mx.col; j++)
		{
			int sum = 0;
			for(int k=0; k < col; k++)
			{
				sum += m[i][k] * mx.m[k][j];	    			
			}	 
			ab.m[i][j] = sum;
		}
	}
	return ab;	     
}


void Array2D::printMatrix ()
{
	for (int i = 0; i < row; i++)
	{
		cout<< "\t [";
		for (int j = 0; j < col; j++)
		{
			if(j+1 != col)
				cout  <<m[i][j] << "\t";
				
			else
				cout  <<m[i][j];
		}
		cout << "]"<< endl;
	}
}


int main()
{
	srand(time(NULL));

			int rows=2, cols=2;
			int rowsB;
			int colsB=1;
						
			rowsB = cols;	 	 	 	 
									
			cout<< "Given matrix A\n\n";	
			Array2D mxA3(rows, cols);
			mxA3.printMatrix ();
								
			cout<< "\nand Array2D B\n\n";
			Array2D mxB3(rowsB, colsB);
			mxB3.printMatrix ();
									
			cout<< "\nA * B is \n\n";
			Array2D mxAB(rows, colsB);
			mxAB = mxA3.multiplyMatrix(mxB3); 
				
			mxAB.printMatrix ();
				

} 

[Duthomhas]

The problem is because you did not implement a copy assignment operator as I instructed you, so when "ab" (line 109) is destructed (line 125), it destroys "mxAB"'s memory too...

To see the difference, change lines 165 and 166 to:

			Array2D mxAB(mxA3.multiplyMatrix(mxB3));

In this case, you have a proper copy constructor implemented, so when "ab" (line 109) is destructed, it only affects itself (since "mxAB" has its own copy of the matrix data).

A few other things you should be aware of:


Line 33: static int size=0; What is this for?


Line 36: Your default constructor needs to initialize the values of your class, otherwise your destructor will crash your program with a GPF.

Array2D::Array2D()
{
	row = col = 0;
	m = NULL;
}

Now, when your destructor is called, make sure you don't try to delete[] anything that doesn't exist:

Array2D::~Array2D()
{
	if (m != NULL)
	{
		for (int i = 0; i < row; i++)
	  	{
  			delete[]m[i];
	  	}
		delete[]m;
	}

	// This isn't actually necessary in the destructor, but keep reading...
	row = col = 0;
	m = NULL;
}

When you implement your assignment operator, keep in mind that you may have to destroy existing data:

Array2D& Array2D::operator = ( const Array2D& a )
{
	if (m != NULL)
	{
		finalize();  // same stuff as in destructor, not forgetting the stuff = 0 statements
	}

	copy( a );  // same stuff as in the copy constructor
}

[edit] You will need to implement the finalize() and copy() methods, just like initialize() and generateArray2D() are.[/edit]

Finally, your matrix class does not make its size or contents available for inspection. You might want to add that kind of stuff:

class Array2D
{
	public:
		...

		int rows()    const { return row; }
		int columns() const { return col; }

		int& index( int r, int c ) { return m[r][c]; }

	...
};

...

int main()
{
	...
	cout << "The size of matrix A is " << mxA3.rows() << " by " << mxA3.columns() << ".\n";
	...
	mxA3.index( 0, 1 ) = 42;
	...

Hope this helps.

[zelta]

Hi! Ya! It works great according to your instructions. However, I've some questions to ask:

1) If I don't have a assignment (=) operator and obtain the erroneous results is because the array mxAB just points to what mxA3 points to (line 165), it doesn't not have a deep copy of entire array, am I right?

2) Copy constructor and assignment (=) operator are also the same, right? Both of them are deep copying of entire array and If I've a copy constructor so that assignment (=) operator is not necessary as I can do like Array2D mxAB(mxA3.multiplyMatrix(mxB3));

3) How can I add in a static int variable inside the destructor function in order to keep track the rows have been deleted? So that I can call/refer to it inside my main function?
(cout cannot be placed inside the destuoctor as I don't want it to be displayed for all times)

4) I'm also give a void pointer to create a 2D array to be familiar with data type void according to my prof. For the initialization, I won't be able to define storage as above (m = new int* [row];) , so I was taught by someone to do as below, is it correct? Any constraints or problems for doing these? Thanks!

// I'm given a void poiter ( void***m ) instead of int **m

void Array2D::initialize()
{
	m = new void**[row];
	
	for(int i=0; i< row; i++)
		m[i] = new void*[col];
}


void Array2D::generateArray2D ()
{
	int *p;
	
	for (int i = 0; i < row; i++)
	{
		for (int j = 0; j < col; j++)
		{
			p = new int;
			*p = rand () % 10;
			m[i][j] = p;
		}
	}		
}

Thanks!

[Duthomhas]

1) Correct.
2) Mmm, close enough.
3) What? Once you destroy the object, it no longer exists. That is, once you are done with a thing, you should not make any references to it any more. Doing so is akin to:

void fooey( int x )
{
  int y = x * 2;
}

int main()
{
  fooey( 12 );
  cout << fooey::y << endl;  // makes no sense -- fooey::y doesn't exist anymore!
}

4) Er, that's insane, but sure...

m[i][j] = (void*)new int;
*(int*)(m[i][j]) = rand() % 10;

or

m[i][j] = (void*)(new int( rand() % 10 ));

You just have to make absolutely sure you don't forget the casts when you want to get data out (and it is a good idea to have them when putting data in) -- and also that each item is a pointer that must at some point be freed.

Good luck!

[zelta]

@Duoas, really appreciate your efforts. Many thanks ^_^"

How long have you been learning C++?

Do you like JAVA? Why would you prefer either one of them?

[Duthomhas]

Glad to be of help.

24 years.

I do not like Java, to put it mildly. [edit] I can use it proficiently, though. [/edit]