Pass Object by Reference error in C++

Question

I have a MatrixTypeheader file which has the following definition : http://pastebin.com/DMzf1wGB

//Add Two Matrices and store result into another matrix
void Add(MatrixType otherMatrix, MatrixType& resultMatrix);

The implementation of the method above is as such :

void MatrixType::Add(MatrixType otherMatrix, MatrixType& resultMatrix)
{   
    cout << "Inside Add func!" << endl;
    cout << "other matrix : " << endl;
    otherMatrix.PrintMatrix();

    for (int i = 0; i < numRows; i++) {
        for (int j = 0; j < numCols; j++) {
            resultMatrix.values[i][j] = values[i][j] + otherMatrix.values[i][j];
        }
        cout << "\n";
        resultMatrix.PrintMatrix();
        cout << "\n";
    }
}

Definition of PrintMatrix() :

void MatrixType::PrintMatrix()
{
    //Pre: None
    //Post: Matrix is printed row wise
    for (int i = 0; i < numRows; i++) {
        cout << "[ ";
        for (int j = 0; j < numCols; j++) {
            cout << values[i][j];
        }
        cout << "]";
        cout << "\n";
    }

}

Now in my Main.cpp I have MatrixType array like this : MatrixType matrixStore[10] to store 10 MatrixType objects. The complete code for Main.cpp is here : http://pastebin.com/aj2eEGVS

int rows = matrixStore[index1].getRowSize();
int cols = matrixStore[index1].getColSize();
cout << "The two matrices can be added!" << endl;
cout << "Computing... " << endl;

//Create Result Matrix and a pointer variable to it 
MatrixType pResultMatrix = MatrixType(rows, cols);
matrixStore[resultIndex] = pResultMatrix;

//Invoke Add function
matrixStore[index1].Add(matrixStore[index2], matrixStore[resultIndex]);

Now when in my Add() function I do otherMatrix.PrintMatrix() it prints out the values of the matrix invoking the Add() function. And due to this => either I do not have reference to the matrix object invoking the method or the matrix object being passed as the parameter!

Also whenever I do PrintMatrix() after I have added values (in the next round of Switch Case), I always get junk values.

Any solution/explanation to this ?

TIA

Answer 1

In short, matrixStore stores object values, not pointers (as you expect).

//Create Result Matrix and a pointer variable to it 
MatrixType pResultMatrix = MatrixType(rows, cols);
matrixStore[resultIndex] = pResultMatrix;

Seems you expect to store pointer variable, but it's not true. MatrixType value is stored in matrixStore.

The result of this is incorrect matrixStore filling (code from your link):

MatrixType matrix = MatrixType(rows, cols);
matrixStore[index] = matrix;

cout << "Address of matrixStore[index] : " << &matrixStore[index] << endl;
cout << "Address of new matrix is : " << &matrix << endl;

int value;
for (int i = 0; i < rows; i++) {
    cout << "Row " << i << " : ";
    for (int j = 0; j < cols; j++) {
        cin >> value;
        matrix.StoreItem(value, i, j);
    }
}

All changes to matrix are lost as matrixStore contains old copy of matrix object.

Solution:

The proper declaration of matrixStore to keep pointers is:

MatrixType* matrixStore[10]

In case you would like to operate with MatrixType value objects you need to copy objects into matrixStore all time you've modified them.

Answer 2

The main problem is not in this part of your code but during the matrix creation/initialization. You are storing objects in the array which can be good (especially I like it because it is created on the stack which is much faster, just be aware to do not create too long array to avoid stack overflow) but you have to consider this overall of your code.

For example in the Main.cpp line 38 you copy the content of matrixobject into the array but after that you modify the matrix object which is different from the object which is in the array! This means that the content of the matrix object in the array has some random values. You should modify directly the object in the array, it does not make any sense to have that temporary one.

For example:

matrixStore[index] = MatrixType(rows, cols);

for (int i = 0; i < rows; i++)
{
    std::cout << "Row " << i << " : ";
    for (int j = 0; j < cols; j++)
    {
        std::cin >> value;
        matrixStore[index].StoreItem(value, i, j);
    }
}

I think after this change the part what you copied here should work because there you are working in the array directly.

Some small suggestions:

1. Pass MatrixType object always as reference if possible. For example, in Add function also the òtherMatrix can be a const ref, your code will be much efficient because the object copy will be not involved. For example:

   void MatrixType::Add(const MatrixType& otherMatrix, MatrixType& resultMatrix);
   

Here the otherMatrix is an input, the resultMatrix is (can be) an output parameter.

2. In C++ there is real bool type. Avoid bool isAddComp ... if (isAddComp != 0)code, just use if (isAddComp), this is the C++ way.

3. I would start to use std::vector instead of common arrays, much more flexible and very useful to learn how can you use it.

4. Personally I would not use ùsing namespace for std it is short and better to read your code (but maybe this is just my code style).

Answer 3

Looking at the code here: http://pastebin.com/aj2eEGVS It looks like the only reason you're creating a local matrix in all your paths, for example:

MatrixType matrix = MatrixType(rows, cols);

is to assign values to the members numRows and numCols. I see you have a commented out SetSize method. I would assume you don't want to do use it because you think the rows and columns shouldn't change after creation.

In that case, your matrixStore should be created as pointers:

MatrixType* matrixStore[10];

Now instead of doing:

MatrixType matrix = MatrixType(rows, cols);
matrixStore[index] = matrix;

You do this:

matrixStore[index] = new MatrixType(rows, cols);

And just use:

matrixStore[index]->StoreItem(value, i, j);

or whatever you want to do with it.

In the end, you just call:

delete matrixStore[index];

For all matrices you have used "new" on. The best way to do this is to assign them to nullptr in the beginning.

MatrixType* matrixStore[10];
for ( unsigned int i = 0; i < 10; ++i )
{
    matrixStore[i] = nullptr;
}

And in the end:

for ( unsigned int i = 0; i < 10; ++i )
{
    if (nullptr != matrixStore[i])
    {
        delete matrixStore[i];
    }
}

Answer 4

Looking at the code in http://pastebin.com/aj2eEGVS

It seems to me you are not initializing the matrix in matrixStore. You are filling the local variable matrix.

MatrixType matrix = MatrixType(rows, cols); // LOCAL VARIABLE

**matrixStore[index] = matrix;** // you are copying the uninitialized matrix

// ...

int value;
for (int i = 0; i < rows; i++) {
    cout << "Row " << i << " : ";
    for (int j = 0; j < cols; j++) {
        cin >> value;
        matrix.StoreItem(value, i, j); // LOCAL VARIABLE
    }
}
cout << endl;
//Print matrix so the use can see
matrix.PrintMatrix();     // PRINT LOCAL VARIABLE