Issue with operator overrides

[Will Gresham]

Hello all,

I have an issue with a class I am writing which is supposed to be able to multiple 2 matrices together.

The issue I am having is not the actual multiplication (That bit works, thanks for the pointers yesterday). The problem, or so I assume, is with my overrides as the correct result is not achieved.

This is the code in question:

class Matrix 
{
	public:
		Matrix(void);
		Matrix(int rows, int cols);
		Matrix(const Matrix& m);
		~Matrix(void);

		Matrix& operator=(const Matrix &rhs);
		Matrix operator* (const Matrix &matrix2);

	private:
		vector<vector<float>> _matrix;
		void Copy(const Matrix& m);
};

I have taken out methods I don't think are causing the issue to save space here.

And the code for them:

// Operator Overloads

Matrix& Matrix::operator=(const Matrix &rhs)
{
	if (this != &rhs)
	{
		Copy(rhs);
	}
	return *this;
}

Matrix Matrix::operator* (const Matrix &matrix2) 
{
	Matrix result;
	result.Resize(NumRows(),matrix2.NumCols());
	int i, j, k;
    float sum;
    for (i = 0; i < 3; i++) 
	{
		for (j = 0; j < 3; j++) 
		{
			sum = 0;
			for (k = 0; k < 3; k++) 
			{
				cout << "i: " << i << " j: " << j << " k: " << k;
				sum += GetM(i, k) * matrix2.GetM(k, j);
				cout << " Sum: " << sum << "\n";
			}
			result.SetM(i, j, sum);
		}
    }
	return result;
}

// Private Methods

void Matrix::Copy(const Matrix& m)
{
	Resize(m.NumRows(), m.NumCols());
	for(int i = 0; i < m.NumRows(); i++)
	{
		for(int j = 0; j < m.NumCols(); j++) 
		{
			SetM(i, j, GetM(i, j));
		}
	}
}

If I input into the program 2 3x3 matrices, and put a breakpoint in at return result; on operator* , the variable result contains the correct matrix, so the program multiplies them correctly.

What happens though, in main() is that it simply outputs 0 in every point of the matrix:

matrix3.Resize(matrix1.NumRows(),matrix2.NumCols());
matrix3 = matrix1 * matrix2;
cout << "\nMultiplied Matrix contains:\n";
for(int i = 0; i < matrix3.NumRows(); i++)
{
	for(int j = 0; j < matrix3.NumCols(); j++)
	{
		cout << "[" << i << "][" << j << "] - " << matrix3.GetM(i, j) << "\n"; 
	}
}

Can anyone give me an idea of where this is going wrong?

[Fbody]

I would check the matrix that's going into your assignment operator. What's happening is result gets returned to the calling scope as a temporary object. That temporary object then becomes the rhs argument for the assignment operator.

It's possible that the temporary object is being destroyed before you are done with it.

I would also take a closer look at what's happening in your Copy function. One of the sub-routines it uses is probably not correct.

[gerard4143]

What's your copy constructor look like?

[Fbody]

What's your copy constructor look like?

I was originally thinking that may have something to do with it too, but I really don't see anywhere that it would be called because all of his arguments are references...

Unless it's being used to copy the result Matrix when the operator function returns...

In that case, the problem would be there, or if he wrote it correctly and used his other member functions.

[Will Gresham]

Thanks for the replies, I have been playing about with breakpoints to see what is passes where, or not as the case may be.

Like I say in my first post, the operator* fuction works as expected, and result contains the correct data.
However, When I get to the breakpoint on operator= both 'this' and 'rhs' contain nothing but 0 as their values, I would expect 'this' to be empty, since it should contain nothing at this point, so it seems to be an issue passing the returned value from the multiply function to the assignment one.

Any ideas on how I can ensure this value isn't destroyed before it is passed to the assignemnt?

I apologise if this is a simple question, I am still fairly new to C++.

[Fbody]

Please post your copy constructor. There may be an issue with that.

If your debugger has the ability to "step" through the execution of your code, it would be a good idea to do so to get a better idea of where things are going awry.

[Will Gresham]

Using Visual Studio 2010, so I can step through.

Here is the copy constructor:

Matrix::Matrix(const Matrix& m)
{
	Resize(m.NumRows(), m.NumCols());
	Copy(m);
}

[Will Gresham]

I found the issue, stupid little mistake, I was copying an empty value to itself :(

Changed this line SetM(i, j, GetM(i, j)); to SetM(i, j, m.GetM(i, j)); and it all works.

Thanks for your input.

[Fbody]

Good. I was starting to think it was a problem with your Matrix::Copy() function.