Problem in copy constructor

[james6754]

Hello I have this problem..I realise I am trying to

I am calling function like this...a reference to E is passed in?

B = D.sum1(E);



MatrixClass MatrixClass::sum1(const MatrixClass &rhs) const//function to add elements of one MatrixClass to
            {                                                           //another and return the new MatrixClass
                MatrixClass C;

                if(this->dataptr == rhs.dataptr)
                {
                    return *this;//check for self assignment
                }

                C.dataptr = new double[M * N];//allocate memory for C

                if(M*N*sizeof(this->dataptr) == M*N*sizeof(rhs.dataptr))
                {
                    for(int i = 0;i < (M*N);i++)
                    {
                        C.dataptr[i] = this->dataptr[i] + rhs.dataptr[i];
                    }   
                }
                else
                {
                    C.dataptr = 0;
                    cout << "MATRIX ARE NOT THE SAME DIMENSION";
                }
                return C;
            }//end function sum1

This returns and calls the copy constructor passing a reference to D?

Copy constructor:

MatrixClass::MatrixClass(const MatrixClass &rhs)//copy constructor
{
    M = rhs.M;
    N = rhs.N;

    this->dataptr = new double[M*N];

    if(rhs.dataptr)
    {
        int num = sizeof(double);
        memcpy(this->dataptr,rhs.dataptr,M*N*sizeof(double));
        //for (int i = 0;i < 20;i++)
        //this->dataptr[i] = rhs.dataptr[i];    
    }
    else
    {
        this->dataptr = 0;
    }
}//end copy constructor

The problem lies when I am trying to allocate memory to 'this->dataptr', I believe that 'this' is not initilized, but I thought that 'this' would refer to 'B' and 'rhs' would refer to 'D'??

Could anyone please help me to see where I am going wrong?

Thanks!

[StuXYZ]

The first problem is that it is very very difficult to figure out without the class definition, so I could be extemely wrong.

(a) I have not idea why you can't add two identical matrixes together, e.g. MatrixClass Sum=A.sum1(A);
So I think lines 9-11 are strange .

(b) The code for both the constructor AND the sum have the following memory leak: [See comments]

MatrixClass MatrixClass::sum1(const MatrixClass &rhs) const
{                                                          
  MatrixClass C;
  C.dataptr = new double[M * N];                      // ALLOCATE HERE

  // This test is wrong, consider that sizeof(rhs.dataptr) is NOT
  // the number of M*N but actually the size of a pointer. 
  if(M*N*sizeof(this->dataptr) == M*N*sizeof(rhs.dataptr))
     { //... }
  else
    {
      C.dataptr = 0;     // MEMORY NOT FREED         
    }
  // C IS A MATRIX CLASS BUT N / M ARE NOT SET CORRECTLY
  return C;
}

You can see that you both have a memory leak and do not set M and N in object C. Since although you allocate the memory, you do not set the sizes. That will have nasty effects later.

Your test should have been : if (M*N==rhs.M*rhs.N) Your actual test make the wrong assumption about what sizeof(rhs.dataptr) returns, which is not a runtime value but a compile time value. It is going to return the size of a pointer [typically 8 for many compilers]