Have I correctly implemented this copy constructor for a dynamically allocated array?

[Shishykish]

Hello,

From my header I have got a warning

Warning C26495 Variable 'ComplexMatrix::Arr' is uninitialized. Always initialize a member variable (type.6)R.H 64

So I just wanted someone's opinion on where the error is within the copy constructor and what I may have made a mistake on?

Header.h

#pragma once
#include <iostream>
#include <complex>
#include <cmath>

using namespace std;

class ComplexMatrix
{
private:
	complex<long double>** Arr;
	int mi = 3;
	int mj = 3;
public:
	ComplexMatrix();
	ComplexMatrix(int i, int j);
	ComplexMatrix(const ComplexMatrix&);
	void Initialise(complex<long double>);
	void DisplayMatrix();
	void DeleteMatrix();
	void EnterComplexMatrix(int, int);
}; 

//Constructor to initialise a default 3 x 3 complex matrix with 0s for real and imaginary values
ComplexMatrix::ComplexMatrix()
{
	Arr = new complex<long double> * [mi];

	for (int x = 0; x < mi; x++)
	{
		Arr[x] = new complex<long double>  [mj];
	}

	for (int x1 = 0; x1 < mi; x1++)
	{
		for (int y1 = 0; y1 < mj; y1++)
		{
			Arr[x1][y1] = (0.0, 0.0);
		}
	} 
}

//Constructor to initialise a user defined complex matrix of a particular size with 0s for real and imaginary values
ComplexMatrix::ComplexMatrix(int i, int j)
{
	mi = i;
	mj = j;
	Arr = new complex<long double> * [i];

	for (int x = 0; x < i; x++)
	{
		Arr[x] = new complex<long double>[j];
	}
	for (int x1 = 0; x1 < i; x1++)
	{
		for (int y1 = 0; y1 < j; y1++)
		{
			Arr[x1][y1] = (0.0, 0.0);
		}
	}
}

//Copy Constructor
ComplexMatrix::ComplexMatrix(const ComplexMatrix& Arr)
{
	complex <long double>** copyArr = new complex<long double> * [mi];

	for (int x = 0; x < mi; x++)
	{
		copyArr[x] = new complex<long double>[mj];
	}

	for (int x1 = 0; x1 < mi; x1++)
	{
		for (int y1 = 0; y1 < mj; y1++)
		{
			copyArr[x1][y1] = Arr.Arr[x1][y1];
		}
	}
}

//Initialise matrix elements to a particular value
void ComplexMatrix::Initialise(complex<long double> x)
{
	for (int i = 0; i < mi; i++)
	{
		for (int j = 0; j < mj; j++)
		{
			Arr[i][j] = x;
		}
	}
}

//Display the matrix member function
void ComplexMatrix::DisplayMatrix()
{
	for (int x = 0; x < mi; x++)
	{
		for (int y = 0; y < mj; y++)
		{
			cout << Arr[x][y];
		}
		cout << endl; 
	}
}

//Delete the memory allocated by the matrix member function
void ComplexMatrix::DeleteMatrix()
{
	for (int x = 0; x < mi; x++)
	{
		delete[] Arr[x];
	}
	delete[] Arr;
}

//Enter complex matrix elements member function
void ComplexMatrix::EnterComplexMatrix(int i, int j)
{
	double real, img;
	complex < long double> temp = (0.0, 0.0);
	cout << "Your matrix will have " << i * j << " elements" << endl;

	//Prompt for user input and assign values for real and imaginary values
	for (int x = 0; x < i; x++)
	{
		for (int y = 0; y < j; y++)
		{
			cout << "Enter the details for the real part of element" << "[" << x << "]" << "[" << y << "]" << endl;
			cin >> real;
			cout << "Enter the details for the real part of element" << "[" << x << "]" << "[" << y << "]" << endl;
			cin >> img;
			temp = (real, img);
			Arr[x][y] = temp;
		}
	}
}

Main

#include <iostream>
#include "Header.h"
#include <complex>
#include <cmath>

using namespace std;

int main()
{
    std::cout << "This program will calculate the exp of a matrix A\n";
    complex<long double> x = {1.1, 2.2};
    complex<long double> y = {3.3, 4.4};

    ComplexMatrix z1(3,3);
    ComplexMatrix z2(3,3);

    z1.Initialise(x);
    z2.Initialise(y);

    z1.DisplayMatrix();
    cout << endl;
    z2.DisplayMatrix();

    z2 = z1;
    z2.DisplayMatrix();

    z1.DeleteMatrix();
    z2.DeleteMatrix();
} 

[Ganado]

In your copy constructor, where do you assign a proper value to Arr?

[jonnin]

try this

//Copy Constructor
ComplexMatrix::ComplexMatrix(const ComplexMatrix& CM) 
//lets only have ARR mean one thing to make it easier to read, understand, and to avoid this-> clutter. 
{
	Arr = new complex<long double> * [mi];

	for (int x = 0; x < mi; x++)
	{
		Arr[x] = new complex<long double>[mj];
	}

	for (int x1 = 0; x1 < mi; x1++)
	{
		for (int y1 = 0; y1 < mj; y1++)
		{
			Arr[x1][y1] = CM.Arr[x1][y1];
		}
	}
}

[Shishykish]

Hello,

Thanks Jonnin! works fine with the following int main. My mistake was to get the Arr passed by reference, rename it Arr and then make a new object called copyArr, and then not bother to initialise Arr that was being waited for by the compiler. I was getting the argument calls the wrong way around.

#include <iostream>
#include "Header.h"
#include <complex>
#include <cmath>

using namespace std;

int main()
{
    std::cout << "This program will calculate the exp of a matrix A\n";
    complex<long double> x = {1.1, 2.2};
    complex<long double> y = { 3.3, 4.4 };

    ComplexMatrix z1(3, 3);
    z1.DisplayMatrix();

    z1.Initialise(x);

    ComplexMatrix z2(z1);
    z2.DisplayMatrix();

    z1.DeleteMatrix();
    z2.DeleteMatrix();
} 

This program will calculate the exp of a matrix A
(0,0)(0,0)(0,0)
(0,0)(0,0)(0,0)
(0,0)(0,0)(0,0)
(1.1,2.2)(1.1,2.2)(1.1,2.2)
(1.1,2.2)(1.1,2.2)(1.1,2.2)
(1.1,2.2)(1.1,2.2)(1.1,2.2)

[jonnin]

you were all over it, just not quite :)
has anyone lectured you on using double* instead of double** or vector<double> instead?
You are making this way too complicated and high risk unless its an exercise to learn or remember why pointers are a lot of trouble. Imagine if you could copy the thing with just a memcpy instead of one at a time in a loop? Imagine if you could transpose in place and flip the row/col values and be done with it? Imagine if to add 2 matrix it was just a for loop? To allocate it, no double loop, to deallocate it, no double loop? Just to name a few starting reasons, there are many, many pros and no major cons.

[Shishykish]

WTH!!!! What kind of C++ black magic is this?

using double* instead of double** or vector<double> instead?
You are making this way too complicated and high risk unless its an exercise to learn or remember why pointers are a lot of trouble. Imagine if you could copy the thing with just a memcpy instead of one at a time in a loop? Imagine if you could transpose in place and flip the row/col values and be done with it? Imagine if to add 2 matrix it was just a for loop? To allocate it, no double loop, to deallocate it, no double loop?

Could you point me in the right direction on these 2 things? Currently, my textbook problem is to work out the exp of a matrix using a formula. It's got a summation, raising to a power and a scalar division all to perform on a complex matrix. My textbook had it's own complex class and I made the matrix class, but it was just too cumbersome to make about 19232 operators so they could talk to each other. So I just opted for the <complex> one instead. Currently this is it.

#pragma once
#include <iostream>
#include <complex>
#include <cmath>

using namespace std;

class ComplexMatrix
{
private:
	complex<long double>** Arr;
	int mi = 3;
	int mj = 3;
public:
	ComplexMatrix();
	ComplexMatrix(int i, int j);
	ComplexMatrix(const ComplexMatrix&);
	void Initialise(complex<long double>);
	void DisplayMatrix();
	void DeleteMatrix();
	void EnterComplexMatrix(int, int);
	ComplexMatrix Exp_by_sq(int);
	ComplexMatrix ScalarDivision(double);
	ComplexMatrix Multiply(ComplexMatrix, ComplexMatrix);
	ComplexMatrix operator*(const ComplexMatrix&);
	ComplexMatrix operator=(const ComplexMatrix&);
	
}; 

//Constructor to initialise a default 3 x 3 complex matrix with 0s for real and imaginary values
ComplexMatrix::ComplexMatrix()
{
	Arr = new complex<long double> * [mi];

	for (int x = 0; x < mi; x++)
	{
		Arr[x] = new complex<long double>  [mj];
	}

	for (int x1 = 0; x1 < mi; x1++)
	{
		for (int y1 = 0; y1 < mj; y1++)
		{
			Arr[x1][y1] = (0.0, 0.0);
		}
	} 
}

//Constructor to initialise a user defined complex matrix of a particular size with 0s for real and imaginary values
ComplexMatrix::ComplexMatrix(int i, int j)
{
	mi = i;
	mj = j;
	Arr = new complex<long double> * [i];

	for (int x = 0; x < i; x++)
	{
		Arr[x] = new complex<long double>[j];
	}
	for (int x1 = 0; x1 < i; x1++)
	{
		for (int y1 = 0; y1 < j; y1++)
		{
			Arr[x1][y1] = (0.0, 0.0);
		}
	}
}

//Copy Constructor
ComplexMatrix::ComplexMatrix(const ComplexMatrix& CM)
//lets only have ARR mean one thing to make it easier to read, understand, and to avoid this-> clutter. 
{
	Arr = new complex<long double> * [mi];

	for (int x = 0; x < mi; x++)
	{
		Arr[x] = new complex<long double>[mj];
	}

	for (int x1 = 0; x1 < mi; x1++)
	{
		for (int y1 = 0; y1 < mj; y1++)
		{
			Arr[x1][y1] = CM.Arr[x1][y1];
		}
	}
}

//Initialise matrix elements to a particular value
void ComplexMatrix::Initialise(complex<long double> x)
{
	for (int i = 0; i < mi; i++)
	{
		for (int j = 0; j < mj; j++)
		{
			Arr[i][j] = x;
		}
	}
}

//Display the matrix member function
void ComplexMatrix::DisplayMatrix()
{
	for (int x = 0; x < mi; x++)
	{
		for (int y = 0; y < mj; y++)
		{
			cout << Arr[x][y];
		}
		cout << endl; 
	}
}

//Delete the memory allocated by the matrix member function
void ComplexMatrix::DeleteMatrix()
{
	for (int x = 0; x < mi; x++)
	{
		delete[] Arr[x];
	}
	delete[] Arr;
}

//Enter complex matrix elements member function
void ComplexMatrix::EnterComplexMatrix(int i, int j)
{
	double real, img;
	complex < long double> temp = (0.0, 0.0);
	cout << "Your matrix will have " << i * j << " elements" << endl;

	//Prompt for user input and assign values for real and imaginary values
	for (int x = 0; x < i; x++)
	{
		for (int y = 0; y < j; y++)
		{
			cout << "Enter the details for the real part of element" << "[" << x << "]" << "[" << y << "]" << endl;
			cin >> real;
			cout << "Enter the details for the real part of element" << "[" << x << "]" << "[" << y << "]" << endl;
			cin >> img;
			temp = (real, img);
			Arr[x][y] = temp;
		}
	}
}

ComplexMatrix ComplexMatrix::Multiply(ComplexMatrix x, ComplexMatrix y)
{
	ComplexMatrix z(3, 3);

	for (int x1 = 0; x1 < 3; ++x1)
	{
		for (int y1 = 0; y1 < 3; ++y1)
		{
			for (int z1 = 0; z1 < 3; ++z1)
			{
				Arr[x1][y1] += x.Arr[x1][z1] * y.Arr[z1][y1];
			}
		}
	}
	return z;
}

ComplexMatrix ComplexMatrix::ScalarDivision(double n)
{
	complex<long double> x;
	x = n;
	ComplexMatrix newCompArr(3, 3);
	for (int x1 = 0; x1 < mi; x1++)
	{
		for (int y1 = 0; y1 < mj; y1++)
		{
			newCompArr.Arr[x1][y1] = Arr[x1][y1] / x;
		}
	}
	return newCompArr;
}

ComplexMatrix ComplexMatrix::Exp_by_sq(int n)
{
	ComplexMatrix y(mi, mj);
	
	if (n == 1)
	{
		for (int i = 0; i < mi; i++)
		{
			for (int j = 0; j < mj; j++)
			{
				y.Arr[i][j] = (1.0, 1.0);
			}
		}
	}
	else if (n % 2 == 0)
	{
		for (int x1 = 0; x1 < (n / 2); x1++)
		{
			y = y * y;
		}
	}
	else
	{
		for (int y1 = 0; y1 < ((n - 1) / 2); y1++)
		{
			y = y * y;
		}
		y = y * y;
	}
	return y;
}

ComplexMatrix ComplexMatrix::operator*(const ComplexMatrix& CompArr)
{
	ComplexMatrix newCompArr(3, 3);
	for (int x1 = 0; x1 < 3; ++x1)
	{
		for (int y1 = 0; y1 < 3; ++y1)
		{
			for (int z1 = 0; z1 < 3; ++z1)
			{
				newCompArr.Arr[x1][y1] += Arr[x1][z1] * CompArr.Arr[z1][y1];
			}
		}
	}
	return newCompArr;
}

ComplexMatrix ComplexMatrix::operator=(const ComplexMatrix& CompArr)
{
	ComplexMatrix newCompArr(3, 3);
	for (int x1 = 0; x1 < mi; x1++)
	{
		for (int y1 = 0; y1 < mj; y1++)
		{
			newCompArr.Arr[x1][y1] = CompArr.Arr[x1][y1];
		}
	}
	return *this;
}

[jonnin]

Could you point me in the right direction on these 2 things?

absolutely.
memory allocated by new is ensured in c++ to be one solid block of ram; that is, each element is an offset from the first with no gaps. But, when you do 2-d, that is not true: the outer layer is a solid block of pointers, and the inner blocks are solid blocks of data, but the inner blocks can be apart because each one was allocated with its own new statement.

if you allocate as one solid block, you can manually index it as if it were 2-d:
you have the number of rows and cols.
int nr = 3;
int nc = 4;
int * matrix = new int[nr*nc]; 12 elements.
Say I want to set [2][2] to 10:
the formula for that is matrix[desired row* number of columns+ desired column] where nc is number of columns.
so
matrix[2*nc+2] = 10;
this can be hidden in your matrix class, you can overload [][] (see https://stackoverflow.com/questions/6969881/operator-overload as this is tricksy) to hide the details.

now that is is one solid block, you can use .read() and .write() for a binary file of your matrix data, or sendto() to transmit it over a network, or memcpy to copy it (you won't do this one if you use vector below), and many other things that open up when using 1-d instead of 2-d.

vectors make it even nicer.
vector<int> matrix(nc*nr);
now you have matrix.size() which is nc*nr (you still need nc and nr, so you know rows and cols, in you class)
now you can increase its size (say, for ax=b RREF solver, append a column)
using push_back() (you still have rearrange the data, probably transpose, add a row, transpose back to add a column).
Now you can copy it:
matrix = othermatrix; //vector has copy built out already, so your copy operator in your class is just data = other.data, numrows = other.numrows, numcols = other.numcols, nothing else, very simple.
and many other things, and it handles all the dynamic memory for you.

[keskiverto]

What if I forget to call DeleteMatrix()?
What if I call DeleteMatrix() twice?
What if I (try to) access elements after calling DeleteMatrix()?

Why there is no appropriate destructor?

ComplexMatrix ComplexMatrix::operator=(const ComplexMatrix& CompArr)
{
	ComplexMatrix newCompArr(3, 3); // newCompArr.mi==newCompArr.mj==3
	for (int x1 = 0; x1 < mi; x1++) // what if mi != 3
	{
		for (int y1 = 0; y1 < mj; y1++) // what if mj != 3
		{
			newCompArr.Arr[x1][y1] = CompArr.Arr[x1][y1];
		}
	}

	return *this; // *this was not modified
	// newCompArr is destructed without calling newCompArr.DeleteMatrix()
}

[Shishykish]

Hello Keskiverto!

Thanks very much. I had to make the code for a 3x3 matrix and these tips will help a lot. I will add some conditional assert checks in the operator as I haven't learnt about exception handling as of yet. My textbook didn't even mention destructors! I will give these all a whirl once I have read into those things.

[Shishykish]

Hi Joninn

I will try doing the code again with the vector library once I get my head around it. But vector vs manual management, won't there be a loss of speed? My reason being that I am interested in making the switch to scientific computing from teaching. I know C++ is faster than most other languages, but writing it using the vector library above might slow things down too much?

[Shishykish]

vectors make it even nicer.
vector<int> matrix(nc*nr);
now you have matrix.size() which is nc*nr (you still need nc and nr, so you know rows and cols, in you class)
now you can increase its size (say, for ax=b RREF solver, append a column)
using push_back() (you still have rearrange the data, probably transpose, add a row, transpose back to add a column).
Now you can copy it:
matrix = othermatrix; //vector has copy built out already, so your copy operator in your class is just data = other.data, numrows = other.numrows, numcols = other.numcols, nothing else, very simple.
and many other things, and it handles all the dynamic memory for you.

This is really cool! Once I nail this version, I will re-write the class with vectors instead.

[jonnin]

Hi Joninn

I will try doing the code again with the vector library once I get my head around it. But vector vs manual management, won't there be a loss of speed? My reason being that I am interested in making the switch to scientific computing from teaching. I know C++ is faster than most other languages, but writing it using the vector library above might slow things down too much?

vectors are high performance. They were slow when they first came out, but over many decades they have been *highly* optimized. That said, you can *make* them inefficient with careless code: just like with your own pointers, you want to avoid allocate and deallocate over and over in a loop or over and over by calling the same function. That is a tweak you can fix once it is working, if you screw it up.

Vector is not doing much that you were not going to have to do yourself, and it does it efficiently. It tracks its size and capacity, which costs a cpu cycle or two (you get ~3 billion of those per second, so keep this in scope... its nothing) and some other little things.

If you do large matrices, you will be using a matrix library, not hand rolled code, for 90% or more of it. Your code is unlikely to be better than a good library; for a simple example, most people hand roll a matrix multiply to be N^3 effort, but there is a better way (its a fair bit of code to do this algorithm while the N^3 is just loops). Also big matrices are prone to page faults, so you have to be careful how you access memory (A*B can be much faster if you transpose and then iterate both across the columns, rather than jumping across large rows repeatedly). Bottom line is there is a lot to it and the matrix libraries have taken most of it into consideration already. Also gotta do a sparse matrix version for many of the algorithms.

Big linear algebra performance is all about the 3 basics... best algorithm for the job, best memory management for the code, and avoidance of excess copying of the same data. Do those 3 things and it will be a long way down the road.

[Shishykish]

Hi Jonnin,

That was really insightful. Thank you! I was a little aware that it was always better to use a good library but didn't realise how well developed they were. I am currently forced by the books that I am learning from that force you down the road of the manual memory management to teach you about it I guess and make you get sick of memory exceptions being thrown at you.

A lot of physics problems are best solved using the techniques you describe above. Ultimately, I would like to change from teaching into scientific computing so these wise words are noted! Thanks.

[jonnin]

That was really insightful. Thank you! I was a little aware that it was always better to use a good library but didn't realise how well developed they were.

Linear algebra is one of the oldest problems on computing, and the libraries for it are very well done and rich. The first widely used versions of LApack (linear algebra package) were around 1990 (trying to recall) and there were elements of it before that in something with a similar (I can't recall now) name.