Dynamically Allocated 2D Matrix Memory Issues

[krispygrimace]

I have written some code to dynamically allocate memory for a 2D matrix. I am using overload operator to perform functions on the matrix naturally. I am having memory issues (Unhandled exception at 0x... Access violation reading ...). Here is my class, header, and main code. The code builds fine with no errors or warnings but I get severe problems when I run it.

// HEADER FILE
#include <iostream>
#include <iomanip>
#include <stdlib.h>

using namespace std;

typedef int p2dArrayData;

class p2dArray
{	
	public:
		p2dArray();
		p2dArray (int rows, int cols);
		
		p2dArray operator+(const p2dArray&);
		p2dArray& operator=(const p2dArray&);

		~p2dArray();
	private:
		int HEIGHT;
		int WIDTH; 
		p2dArrayData **contents;
};

// Matrix Class
#include <iostream>
#include <iomanip>
#include <stdlib.h>
#include "p2dArray.h"
using namespace std;

p2dArray::p2dArray()
{
}

p2dArray::p2dArray(int ROWS_Given, int COLS_Given)
{
	HEIGHT = ROWS_Given;
	WIDTH = COLS_Given;
	contents = new p2dArrayData* [ROWS_Given];

	for (int i = 0; i < ROWS_Given; i++) {
		contents[i] = new p2dArrayData [COLS_Given];
	}

	for(int i = 0; i < ROWS_Given; i++) {
		for(int j = 0; j < COLS_Given; j++) {
			contents[i][j] = (p2dArrayData) i * j;
		}
	}
}

// Deep Copy Constructor
p2dArray& p2dArray::operator=(const p2dArray& operand) 
{
	if(this != &operand) {
		for(int i = 0; i < operand.HEIGHT; i++) {
			for(int j = 0; j < operand.WIDTH; j++) {
					contents[i][j] = operand.contents[i][j];
			}
		}
	}	
	return *this;
}

// Overloaded + Operator
p2dArray p2dArray::operator+(const p2dArray& operand)
{
	p2dArray temp2d(HEIGHT, WIDTH);
	if(HEIGHT != operand.HEIGHT && WIDTH != operand.WIDTH) throw(1);
	for(int i = 0; i < HEIGHT; i++) {
		for(int j = 0; j < WIDTH; j++) {
			temp2d.contents[i][j] = contents[i][j] + operand.contents[i][j];
		}
	}
	return temp2d;
} 

// Destructor
p2dArray::~p2dArray()
{
	for (int i = 0; i < HEIGHT; i++)
		delete[] contents[i];
	delete [] contents;
}

// MAIN FILE
#include <iostream>
#include <iomanip>
#include <stdlib.h>
#include "p2dArray.h"

using namespace std;

int main()
{
	p2dArray first(2, 3);
	p2dArray second(2, 3); 

	first = second + first;

	return 0;
}

I believe the problem may be in my copy constructor. However, if I remove the destructor, I get no errors so I might not be doing that right either. I am at a loss.

[ArkM]

Contrary to your source comments, the class p2dArray has no proper copy constructor(s). You define (wrong) assignment operator but don't define explicit copy constructor. A copy constructor must have (as usually) the signature:

p2dArray::p2dArray(const p2dArray&);

No such member declared in your class but default copy contructor (copy member by member) is inadequate to the dynamic nature of the contents pointer member. However return temp2d; statement in operator+ needed a copy constructor. Inadequate default copy constructor copies temp2d.contents pointer to the result matrix then temp2d object is destructed. Now result object has a pointer to deallocated memory.

That's a brief story.

Yet another remarks:
- The class has inadequate default constructor. It does not initialize members and you get unpredictable pointers and dimensions in constructed objects.
- The operator= does not check target dimensions.

Moral: feel the difference between assignment op and copy constructor...

[krispygrimace]

I am fairly new to c++ and programming in general but these are my attempts at fixing the problem based on your comments. I have also attached my full code for all files. I don't get the same error as I did before, I get a new one. It says "unhandled exception at 0x...". It then points a line in a file called dbgdel.cpp that says.

/* verify block type */
_ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse));

Here are my main changes:Contrary to your source comments, the class p2dArray has no proper copy constructor(s). You define (wrong) assignment operator but don't define explicit copy constructor.I added this as a deep copy constructor

// Deep Copy Constructor
p2dArray::p2dArray(const p2dArray& operand) 
{
	for (int i = 0; i < HEIGHT; i++)
		delete[] contents[i];
	delete [] contents;

	contents = new p2dArrayData* [operand.HEIGHT];
	for (int i = 0; i < operand.HEIGHT; i++) {
		contents[i] = new p2dArrayData [operand.WIDTH];
	}
	HEIGHT = operand.HEIGHT;
	WIDTH = operand.WIDTH;
	for (int i = 0; i < operand.HEIGHT; i++) {
		for (int j = 0; j < operand.WIDTH; j++) {
			contents[i][j] = operand.contents[i][j];
		}
	}
}

The class has inadequate default constructor. It does not initialize members and you get unpredictable pointers and dimensions in constructed objects.

I initialized members in the default constructor

p2dArray::p2dArray() : HEIGHT(0), WIDTH(0)
{
}

The operator= does not check target dimensions.

I added a try throw catch statement to check dimensions.

[ArkM]

I see you did not feel the difference ;)...

An assignment operator has a valid, initialized target object. It changes the contents of this object. A copy constructor has raw memory chunk (the same as any other constructor). It must build the object with the same content as its argument from the scratch. Alas, your copy constructor starting from the deallocation of never allocated memory (and the program gets memory exception, of course).

Look at your new copy contructor body. You delete previous contents - but NO any previous contents here! You must allocate memory for this new matrix (see p2dArray(int,int) constructor) then copy argument's data to this allocated 2-d array.

In the new default constructor you forgot to initialize the most sensitive member: the contents pointer. Therefore the constructed (empty) object has garbage valued pointer (instead of the valid null pointer). Now let's look at the class destructor. What happens when it will try to destruct an empty object with unpredictable garbage value of the contents member? Right, it will try to deallocate this "pointer to nowhere"...

It's up to you...

[krispygrimace]

Thanks a bunch! I know understand what a copy constructor is.