Ian
Ian

Reputation: 659

Copy constructor fails and succeeds in identical circumstances

I am writing a matrix class for a math project, and I have a strange problem where the copy constructor I've written fails in one function but succeeds in another- and the circumstances are seemingly identical. I've run it through gdb and the copy constructor executes but somehow just doesn't assign the values after it finishes.

Here is the output from gdb:

(gdb) break main.cpp:20
Breakpoint 1 at 0x8048913: file main.cpp, line 20.
(gdb) run
Starting program: /home/ian/Documents/math471/src/compress 
printing
1 1 1 1 
1 2 3 4 
1 3 5 7 
1 4 7 10 
printing
1 1 1 1 
1 2 3 4 
1 3 5 7 
1 4 7 10 
printing
1 1 1 1 
1 2 3 4 
1 3 5 7 
1 4 7 10 
printing
1 1 1 1 
1 2 3 4 
1 3 5 7 
1 4 7 10 

Breakpoint 1, main (argc=1, argv=0xbffff334) at main.cpp:20
20          tridiagonalize(A);
(gdb) s
tridiagonalize (A=...) at tridiagonalize.cpp:10
10          Matrix result(A);
(gdb) print result
$1 = {transposed = false, height = 3903476, width = 0, data = 0x0}
(gdb) s
Matrix::Matrix (this=0xbffff23c, A=...) at matrix.cpp:176
176         height = A.getHeight();
(gdb) print height
$2 = 0
(gdb) n
177         width = A.getWidth();
(gdb) print height
$3 = 4
(gdb) n
178         data = new double*[height];
(gdb) print width
$4 = 4
(gdb) n
179         for(int i = 0; i < height; i++){
(gdb) break matrix.cpp:185
Breakpoint 2 at 0x80492ba: file matrix.cpp, line 185.
(gdb) c
Continuing.

Breakpoint 2, Matrix::Matrix (this=0xbffff23c, A=...) at matrix.cpp:185
185         transposed = false;
(gdb) n
186 }
(gdb) print transposed
$5 = false
(gdb) print this
$6 = (Matrix * const) 0xbffff23c
(gdb) print this->height
$7 = 4
(gdb) n
tridiagonalize (A=...) at tridiagonalize.cpp:11
11          int n = A.getWidth();
(gdb) print result
$8 = {transposed = false, height = 3903476, width = 0, data = 0x0}
(gdb) 

As you can see, the values in result are the same before the constructor as after.

#include "matrix.h"
#include "tridiagonalize.h"


using namespace std;

void function(Matrix &A);

int main(int argc, char** argv){

        Matrix A(4, 4);
        for(int i = 0; i < 16; i++){
                A.set(i/4, i%4, (i%4)*(i/4)+1);
        }
        function(A);
        Matrix B = A;
        A.print();
        B.print();

        tridiagonalize(A);
        //A.print();

        return 0;
}

void function(Matrix &A){
        Matrix result(A);
        A.print();
        result.print();
}

Matrix tridiagonalize(Matrix &A){

        Matrix result(A);
        int n = A.getWidth();
        cout << "width: " << n << endl;
        cout << "width of result: " << result.getWidth() << endl;
            return result;
}

EDIT: here is the matrix code:

void Matrix::set(int i, int j, double value){
        if(transposed){
                int tmp = i;
                i = j;
                j = tmp;
        }
        if(i < 0 || j < 0 || i >= height || j >= width){
                cout << "trying to set value outside of matrix" << endl;
                return;
        }
        if(height > 0 && width > 0){
                data[i][j] = value;
        }
        return;
}

void Matrix::print() const{
        cout << "printing" << endl;
        if(!transposed){
                for(int i = 0; i < height; i++){
                        for(int j = 0; j < width; j++){
                                cout << data[i][j] << " ";
                        }
                        cout << endl;
                }
        }
        else{
                for(int i = 0; i < width; i++){
                        for(int j = 0; j < height; j++){
                                cout << data[j][i] << " ";
                        }
                        cout << endl;
                }
        }
        return;
}

Matrix::~Matrix(){
        //cout << "deleting Matrix" << endl;
        if(data != NULL){
                for(int i = 0; i < height; i++){
                        //cout << data[i] << endl;
                        delete[] data[i];
                }
                //cout << data << endl;
                delete[] data;
        }
}

double Matrix::get(int i, int j) const {
        if(transposed){
                int tmp = i;
                i = j;
                j = tmp;
        }
        if(data != NULL && i >=0 && j >= 0 && i < height && j < width){
                return data[i][j];
        }
        else{
                cout << "error in get" << endl;
        }
        return -1;
}

int Matrix::getHeight() const {
        if(transposed){
                return width;
        }
        else{
                return height;
        }
}

int Matrix::getWidth() const {
        if(transposed){
                return height;
        }
        else{
                return width;
        }
}



Matrix::Matrix(const Matrix& A){
        height = A.getHeight();
        width = A.getWidth();
        data = new double*[height];
        for(int i = 0; i < height; i++){
                data[i] = new double[width];
                for(int j = 0; j < width; j++){
                        data[i][j] = A.get(i,j);
                }
        }
        transposed = false;
}

Matrix Matrix::operator= (Matrix &B){
        if(data != NULL){
                for(int i = 0; i < height; i++){
                        delete[] data[i];
                }
                delete[] data;
        }
        data = NULL;
        Matrix result(B);
        return result;
}

The tridiagonalize function and the 'function' function are set up the same, but function() works and tridiagonalize() doesn't. Any ideas why this is happening?

EDIT: added the matrix code

Upvotes: 0

Views: 148

Answers (1)

sth
sth

Reputation: 229754

Matrix::operator=() isn't working correctly, it frees the old data array but then doesn't create a new one with the data that should be assigned.

It should rather look like this:

Matrix& Matrix::operator= (const Matrix &B){
   // Don't do anything if B is the same object
   if (&B == this)
      return *this;

   // delete data, as in the current version
   if(data != NULL){
      ...
   }

   // Create new array of arrays, fill with numbers from B
   data = new double*[B.getHeight()];
   ...

   return *this;
}

Upvotes: 2

Related Questions