Segmentation Fault with a matrix class

[dert45]

Hi everyone. I am writing a program using dynamic 2D arrays to manipulate matrices. I am currently overloading the operators and I am getting segmentation faults. I have a feeling the problem may be the same thing in all of them and if I get help fixing one I could fix the others, but I will post all three of them anyways.

Here is my class:

class matrixType{
  friend ostream& operator<< (ostream&, const matrixType&);
  friend istream& operator>> (istream&, matrixType&);

public:
  void printSize();
  const matrixType& operator= (const matrixType &right);
  void setMatrix();
  matrixType(int rows = 10, int columns = 10);
  matrixType(const matrixType &copy);
  matrixType operator+ (matrixType right);
  matrixType operator- (matrixType right);
  matrixType operator* (matrixType right);

private:
  int **matrix;
  int rowSize;
  int columnSize;
};

Here is the operator+ overload function

matrixType matrixType::operator+ (matrixType right){
  matrixType matrixSum;

  if(columnSize	!= right.columnSize || rowSize != right.rowSize)
    cout << "These two matrices may not be added together because they are not the same size" << endl;

  matrixSum.matrix = new int*[columnSize];
  for(int n = 0; n < columnSize; n++)
    matrixSum.matrix[n] = new int[rowSize];

  for(int i = 0; i < columnSize; i++){
    for(int j = 0; j < rowSize; j++)
      matrixSum.matrix[i][j] = (matrix[i][j] + right.matrix[i][j]);
      }
  return matrixSum;
}

here is my operator- overloading function

matrixType matrixType::operator- (matrixType right){
  matrixType matrixEnd;
  matrixEnd.matrix = new int*[columnSize];
  for(int n = 0; n < columnSize; n++)
  matrixEnd.matrix[n] = new int[rowSize];

  if(columnSize== right.columnSize && rowSize == right.rowSize){
    for(int i =0; i < columnSize; i++){
      for(int j= 0; j < rowSize; j++)
        matrixEnd.matrix[i][j] = matrix[i][j] - right.matrix[i][j];
    }
  }
  else
    cout << "Matrices are not the same size. They can not be subtracted from eachother."<< endl;

  return matrixEnd;
}

and finally here is my operator* overloading function

matrixType matrixType::operator* (matrixType right){
  matrixType matrixProduct;
  int sum = 0;
  matrixProduct.matrix = new int*[matrixProduct.columnSize];
  for(int m = 0; m < matrixProduct.columnSize; m++)
  matrixProduct.matrix[m] = new int[matrixProduct.rowSize];

  if(rowSize == right.columnSize){
    matrixProduct.rowSize = right.rowSize;
    matrixProduct.columnSize = columnSize;
    for (int i = 0; i < matrixProduct.columnSize; i++){
      for(int j = 0; j < matrixProduct.rowSize; j++){
        for(int n = 0; n < rowSize; n++)
          sum = sum + (matrix[i][n] * right.matrix[n][j]);
        matrixProduct.matrix[i][j] = sum;
        sum = 0;
      }
    }
  }
  else
    cout << "Matrices are not the right sizes to be multiplied." << endl;

  return matrixProduct;
}

I have a feeling it has something to do with setting up my dynamic arrays but I have been tinkering around with it for hours now and can not figure it out. It will compile, but the first time I call on one in the main function it dumps the core due to a segmentation fault. Any help is much appreciated. Thank you!

[kbw]

You seem to have a constructor, but manually construct the temporary matrix manually. You are certainly missing a copy constuctor and assignment operator. You'll need those along with a destructor as you're manipulating dynamic memory.

What does your constructor code and test program look like? I'd like to see how the test matrices are constructed.

[dert45]

I do have a copy constructor right here:

matrixType::matrixType (const matrixType &copy){
  columnSize = copy.columnSize;
  rowSize = copy.rowSize;

  matrix = new int*[columnSize];
  matrix[columnSize] = new int[rowSize];

  for(int i = 0; i < columnSize; i++)
    for(int j = 0; j < rowSize; j++)
      matrix[i][j] = copy.matrix[i][j];
}

The assignment operators have been overloaded and work fine.
My other constructor is this one (the assignment calls for the default size to be 10x10):

matrixType::matrixType (int rows, int columns){
  columnSize = columns;
  rowSize = rows;
  matrix = new int*[columnSize];

  for (int i = 0; i < columnSize; i++){
    matrix[i] = new int[rowSize];
     for(int j = 0; j < rowSize; j++)
       matrix[i][j] = 0;
       }
}

my matrices are constructed from a data file. The sizes are input correctly and the three matrices that I do assign from the data file. There are two more matrices that I have the correct column size and row size but i need them to have their data input by either adding two of the matrices or multiplying two of them.

Here is my main function as of now:

int main(){
  matrixType m1, m2, m3, m4, m5;
  char ch;

  m1.setMatrix();
  m2.setMatrix();
  m3.setMatrix();
  m4.setMatrix();
  m5.setMatrix();

  cin.get(ch);
  cin >> m1;
  cin >> m2;
  cin >> m4;

  m3 = m1;
  cout << m3;

  cout << "working up to this point" << endl;
  m1 + m3;
  /*                                                                                                                                                                                                      
  cout << "working here too" << endl;                                                                                                                                                                     
                                                                                                                                                                                                          
  m5 = m1 * m4;                                                                                                                                                                                           
                                                                                                                                                                                                          
  cout << "First Matrix:" << endl;                                                                                                                                                                        
  cout << m1;                                                                                                                                                                                             
  cout << endl;                                                                                                                                                                                           
  */
  return 0;
}

So if what i think you're saying is right, because i have the constructors, I don't need to dynamically create the 2-d array because it is done in the constructor?

**EDIT** I am trying to work off of that clue but it still doesnt seem to be working. Any tips on how i might set up an operator overload in this situation?

[kbw]

That's good. Let's take a look at operator+.

// you pass by value, invoking a copy; should be const ref 
matrixType matrixType::operator+ (matrixType right){
  // why not specify the size on construction?
  matrixType matrixSum;

  // you can throw an exception if the size is wrong
  if(columnSize	!= right.columnSize || rowSize != right.rowSize)
    cout << "These two matrices may not be added together because they are not the same size" << endl;

  // you've already got an initialised matrix, leaking the original matrix
  matrixSum.matrix = new int*[columnSize];
  for(int n = 0; n < columnSize; n++)
    matrixSum.matrix[n] = new int[rowSize];

  for(int i = 0; i < columnSize; i++){
    for(int j = 0; j < rowSize; j++)
      matrixSum.matrix[i][j] = (matrix[i][j] + right.matrix[i][j]);
      }

  // invoke the copy constructor
  return matrixSum;
}

I can see the memory leak, but I don't see why it crashes. What does setMatrix() do?