Debugging My Matrix Class Implementation

[Jennifu]

Hey all! I'm new here. I actually joined for physics help a couple of days ago, but I found the answer by searching through old threads and didn't have to post a new one. So here's my first post.

Anyway, so I procrastinated on my homework, and now I'm having trouble with it and no longer have time to ask my professor for help before it's due. I'm only in Intro to Comp. Sci 2, so hopefully there are plenty of people who can help me here!

I'm supposed to implement a matrix class. The professor has given us the class declaration and driver. Mine is compiling, but it's freezing when it executes. On the site where we are supposed to upload and test our code, this is the error message that is being displayed: "bash: line 1: 8948 Segmentation fault ./prog2.exe"

Ok, now here's the code. The professor has provided this driver:

#include <iostream>
#include <iomanip>
#include "Matrix.h"

using namespace std;

ostream& printMatrix(Matrix m, ostream& out);
void TestFunction1(Matrix, Matrix, Matrix);



int main(void)
{

//check to see if the static variable matrixCount has been created and init

    cout << "matrixCount = " << Matrix::get_matrixCount() << endl;
    cout << "(matrixCount should be zero)\n\n";


//create 3 matrices, check matrixCount, .and print matrix dimension

    Matrix A(2,3),B(2,3),C(3,3);

    A = B;

    cout << "Matrix A\n";
    cout << "\t Rows = " << A.getRows()    << endl;
    cout << "\t Cols = " << A.getColumns() << endl;

    cout << "Matrix B\n";
    cout << "\t Rows = " << B.getRows()    << endl;
    cout << "\t Cols = " << B.getColumns() << endl;

    cout << "Matrix C\n";
    cout << "\t Rows = " << C.getRows()    << endl;
    cout << "\t Cols = " << C.getColumns() << endl;


    cout << endl;
    cout << "matrixCount = " << Matrix::get_matrixCount() << endl;
    cout << "(matrixCount should be three (3))\n\n";

//Test the copy constructor and destructor
    cout << endl;
    cout << "matrixCount = " << Matrix::get_matrixCount() << endl;
    cout << "(before call the TestFunction1)\n\n";

    TestFunction1(A,B,C);

    cout << endl;
    cout << "matrixCount = " << Matrix::get_matrixCount() << endl;
    cout << "(after call the TestFunction1)\n\n";


//create some data arrays

	int row, col, index;
	int data1[]={1,2,3,4,5,6};
	int data2[]={10,20,30,40,50,60};
	int data3[]={-5,-10,-15,0,5,10,15,36,42};



//Fill Matricies A, B will initial values
//Test setElement
	index = 0;
	for(row = 0; row < 2; row++)
	  for(col = 0 ; col < 3; col++)
	  {
	    A.setElement(row, col, data1[index]);
	    B.setElement(row,col,data2[index]);
	    index++;
	  }

//Fill Matricies C will initial values
//Test overloaded []
    index = 0;
	for(row = 0; row < 3; row++)
	  for(col = 0 ; col < 3; col++)
	  {
	    //C.setElement(row, col, data3[index++]);
	    C[row][col] = data3[index++];
	  }


//Print Matricies A, B, C

	cout<<"Matrix A\n\n";
	printMatrix(A,cout);
    cout << endl << endl;

	cout<<"Matrix B\n\n";
	printMatrix(B,cout);
    cout << endl << endl;

	cout<<"Matrix C\n\n";
	printMatrix(C,cout);
    cout << endl << endl;



//Addition

	cout<<"Matrix A + B\n\n";
	printMatrix(A.add(B),cout);
    cout << endl << endl;




//Test Subtraction

	cout<<"Matrix A - B\n\n";
	printMatrix(A.subtract(B),cout);
    cout << endl << endl;




//Test Multiplication

 	cout<<"Matrix A * C\n\n";
	printMatrix(A.multiply(C),cout);
    cout << endl << endl;


	return 0;
}




//Function to print a Matrix

ostream& printMatrix(Matrix m, ostream& out)
{
    int row, col;
    int r,c;

    row = m.getRows();
    col = m.getColumns();

  	for(r = 0; r < row; r++)
	{
	  for(c = 0 ; c < col; c++)
	    out <<setw(4)<< m.getElement(r, c)<<" ";
	  out<<endl;
	}

    return out;
}


//Test Function 1, test copy constructor
void TestFunction1(Matrix x, Matrix y, Matrix z)
{
    cout << endl;
    cout << "matrixCount = " << Matrix::get_matrixCount() << endl;
    cout << "(In TestFunction1)\n\n";


   return;
}

Here is my header file, Matrix.h:

#ifndef _MATRIX_
#define _MATRIX_

class Matrix
{
    public:
        Matrix(int r=0, int c = 0);
        Matrix(const Matrix& m);
        ~Matrix();
        Matrix operator= (Matrix);
        int* operator[] (int);
        int getRows() const;
        int getColumns() const;
        int getElement(int r, int c) const;
        void setElement (int r, int c, int v);
        Matrix add (const Matrix& m) const;
        Matrix subtract (const Matrix& m) const;
        Matrix multiply (const Matrix& m) const;
        static int get_matrixCount ();
    private:
        int rows, cols;
        int **element;
        static int matrixCount;
};


#endif //_MATRIX_

And here are the functions for the class:

#include "Matrix.h"

int Matrix::matrixCount = 0;

Matrix::Matrix(int r, int c)
{
    if (r > 0 && c > 0)
    {
        rows = r;
        cols = c;
    }
    else
    {
        rows = 0;
        cols = 0;
    }

    element = new int* [rows];

    for (int i=0; i<rows; i++)
    {
        element[i] = new int[cols];
        for (int n = 0; n<cols; n++)
        {
            element[i][n] = 0;
        }
    }
    matrixCount++;
}

Matrix::Matrix(const Matrix& m)
{
    for (int i=0; i<rows; i++)
    {
        delete []element[i];
    }

    delete []element;

    rows = m.rows;
    cols = m.cols;

    element = new int* [rows];

    for (int i=0; i<rows; i++)
    {
        element[i] = new int[cols];
        for (int n = 0; n<cols; n++)
        {
            element[i][n] = m.element[i][n];
        }
    }

}

Matrix::~Matrix()
{
    for (int i=0; i<rows; i++)
    {
        delete []element[i];
    }

        delete []element;
}


Matrix Matrix::operator=(Matrix m)
{
    for (int i=0; i<rows; i++)
    {
        delete []element[i];
    }

    delete []element;

    rows = m.rows;
    cols = m.cols;

    element = new int* [rows];

    for (int i=0; i<rows; i++)
    {
        element[i] = new int[cols];
        for (int n = 0; n<cols; n++)
        {
            element[i][n] = m.element[i][n];
        }
    }

    return *this;
}

int* Matrix::operator[] (int n)
{
    return element[n];
}

int Matrix::getRows() const
{
    return rows;
}

int Matrix::getColumns() const
{
    return cols;
}

int Matrix::getElement(int r, int c) const
{
    return element[r][c];
}


void Matrix::setElement (int r, int c, int v)
{
    if (r>=0 && r<=rows && c>=0 && c<=cols)
        element[r][c] = v;
}

Matrix Matrix::add (const Matrix& m) const
{

    for (int i=0; i<rows; i++)
    {
        for (int n = 0; n<cols; n++)
        {
            element[i][n] += m.element[i][n];
        }
    }

    return *this;
}

Matrix Matrix::subtract (const Matrix& m) const
{
    for (int i=0; i<rows; i++)
    {
        for (int n = 0; n<cols; n++)
        {
            element[i][n] -= m.element[i][n];
        }
    }

    return *this;
}

Matrix Matrix::multiply (const Matrix& m) const
{
    Matrix product;

    product.rows = rows;
    product.cols = m.cols;

    for (int i=0; i<product.rows; i++)
    {
        for (int n = 0; n<product.cols; n++)
        {
            for (int m=0; m<cols; m++)
            {
                product.element[i][n] += element[m][m];
            }
        }
    }

    return product;
}

int Matrix::get_matrixCount()
{
    return matrixCount;
}

Can anyone figure out what's happening? I'm feeling kind of stupid right now, because I've been trying to figure it out forever. Oh, and if you see anything else I might need to fix, can you tell me that as well? Thanks!

 

[Math Is Hard]

hey Jen, I am not going to have a chance to look at this, but post your specific compiler and version info for the experts who will.
I run into problems all the time just between working in MS Visual C++ 6 and .NET.

 

[Jennifu]

I usually work in Code::Blocks v1.0, but I also tried this on Dev-C++ 4.9.9.2. Also, it doesn't work on the site where I upload it (I have no idea how my professor has programmed that site).

 

[Hurkyl]

Your operator= really ought to take a const Matrix& as its argument and return a Matrix&.

That's not the bug you're seeing, but it's triggering one of your bugs too early. Once you make that fix, the driver program will help you find your other bugs. I'll give you a chance to spot them before telling you what they are.

By the way, surely your program gave some output before freezing? That output really helps track down bugs, you should state what output you got when posting errors like this. (In fact, when I'm not using a debugger, I track down bugs by inserting lots and lots of output statements, so I can pin down where the program is failing)

 

[Hurkyl]

Oh, and I think there's one extremely pedantic error you've made -- though you will almost never run into real problems because of it... your header guard shouldn't have a leading underscore. _MATRIX_ is (technically) bad: but MATRIX or MATRIX__ or something like that is fine.

 

[Jennifu]

Thanks for you help!

I fixed the things you mentioned, and it's definitely helped. I haven't yet found the other bugs, though.

Anyway, the output I'm getting is this:

matrixCount = 0
(matrixCount should be zero)

Matrix A
Rows = 2
Cols = 3
Matrix B
Rows = 2
Cols = 3
Matrix C
Rows = 3
Cols = 3

matrixCount = 3
(matrixCount should be three (3))matrixCount = 3
(before call the TestFunction1)

So I'm assuming it's something to do with TestFunction 1, because that is where it is freezing. Is it something to do with my copy constructor?

 

[Hurkyl]

Yep, your copy constructor is a problem. What is your copy constructor doing? Are all of those things appropriate for an object which does not yet exist?

 

[Jennifu]

OOOOOH! ok, I got it. And I also fixed my addition, subtraction, and multiplication functions, which were wrong. I think it's working now, unless there's something else you saw.

Thank you very much!

 

[nmtim]

Oh, and I think there's one extremely pedantic error you've made -- though you will almost never run into real problems because of it... your header guard shouldn't have a leading underscore. _MATRIX_ is (technically) bad: but MATRIX or MATRIX__ or something like that is fine.

Why is that a problem?

 

[verty]

Possibly could cause a linking error perhaps?

 

[D H]

Use the debugger whenever you see an error like the one you had (segmentation fault).

In fact, use the debugger, period. The only way to make sure your code is truly doing the right thing is to step the debugger through each line of code. Developers often have to certify that they did so in rigorous software development environments.

By the way, surely your program gave some output before freezing?

Segmentation faults often cause buffered output to disappear. One way around this is to unbuffer standard output and standard error.

 

[Hurkyl]

Why is that a problem?

Any symbol beginning with two underscores, or beginning with an underscore and a capital letter are reserved for the implementation of the C library. So, technically, using _MATRIX_ in that way could run into undefined behavior should the C library decide to do something with that symbol.

(there's a similar problem with a symbol beginning with an underscore and a lower case letter, but it's not as restrictive)

But this is one of those things that will rarely cause a problem -- I just bring it up because it does cause problems occasionally.

Use the debugger whenever you see an error like the one you had (segmentation fault).

I agree, a debugger is a really good thing to use.

Segmentation faults often cause buffered output to disappear. One way around this is to unbuffer standard output and standard error.

std::endl is supposed to flush the buffer, so we should have at least seen the first line. Many systems line buffer stdout, so we should have seen the second line too.

 

[Jennifu]

I'm just learning to use the debugger, and unfortunately, I learned it during lab on a different compiler. I tried it for that last assignment, but on both Code::Blocks and Dev C++, it kept saying something like there was no debugging info. I tried to follow the instructions to fix it, but couldn't figure it out. Any tips?

And yeah, when I couldn't get it to work, the first two lines of output were displayed before it froze.