-1

this is my first time of using stackoverflow, nice to meet you all. I am writing a matrix class that overloads assignments,addition, subtraction...etc. The assignment operator "=" overloading part seems to work fine, but if I attempt to overload the addition "+" operator, I get error message like this:

"Debug Assertion Failed! Program:...nts\C++\week6....\week6matrix.exe

File: f:\dd\vctools\crt_bld\self_x86\crt\src\dbgdel.cpp

Line:52

Expression:_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)"

But if I remove "delete[]matrix" in my destructor, everything seems to work, but due to the requirement of this project, I needed to have this term in my destructor.

I would have posted a picture but its my first time of using this website, so I don't have the reputation to do that yet so I apologise if my question doesn't seem to make sense, but I ll try my best to explain it if you have any further questions regarding my question.

Thank you.

#include<iostream>
#include <stdlib.h>
using namespace std;
//dynamic matrix
class dymatrix
{
  friend ostream & operator << (ostream &os, dymatrix &om);
  friend istream & operator >> (istream &is, dymatrix &om);
private:
  int rows;
  int columns;
  double *matrix;
public:

  dymatrix(){cout<<"Default constructor called"<<endl; columns = 0; rows=0; matrix=0;}
  dymatrix(int inrows, int incolumns)
  {
    rows = inrows;
    columns = incolumns;
    matrix = new double [inrows*incolumns];
    for (int i=0; i<inrows*incolumns; i++) 
    { 
        matrix[i]=0;
    }
  }
  int lengthr() const {return rows;}  //Returns number of rows.
  int lengthc() const {return columns;}   //Return number of columns.
  dymatrix& operator=(dymatrix&);
  ~dymatrix(){cout<<"Destructor called"<<endl;delete[] matrix;}  

  int index(int i, int j)  //This member function returns the position of each index.
  {
    if (j > 0 && j <=rows && i > 0 && i <=columns)
    {
        return (i-1)+(j-1)*columns;
    }
    else {cout<<"Error, out of range"<<endl; exit (1);}
  }
  double & operator()(int i, int j) {return matrix[index(i,j)];}  //The operator () returns the position of j and i in 1D array.

  dymatrix operator + (dymatrix &arr)  //overloading addition.
  {   

    if (rows !=arr.rows && columns != arr.columns)
    {
        cerr<<"SIZE DO NOT MATCH, YOU FAIL"<<endl; exit(1);
    }
    dymatrix new_matrix(rows,columns);
    for (int j = 0; j < arr.rows*arr.columns; j++)
    {
        //for (int i = 1; i <= arr.columns; i++)
        //{
        //cout<<"****"<<j<<endl;    
        new_matrix.matrix[j]= matrix[j]+arr.matrix[j]; //Putting in the data into this dynamic array for each element.
        //}
    }
    return new_matrix;
  }

};  //Class end.
  dymatrix & dymatrix::operator = (dymatrix &arr) //Overloading "=" operator.
  {
    if (&arr == this) return *this; //If the array is the same, no need to change, just to print. The key word "this" is a pointer to the object, and *this gives the object.
    delete[] matrix; matrix =0; rows =0; columns =0;
    rows = arr.rows;   //Setting row length.
    columns = arr.columns;    //Setting column length.
    if(rows*columns > 0)
    {
        matrix = new double [rows*columns]; //Defining a dynamic array here.
        for (int j = 1; j <= rows; j++) //Assigning each term to each term.
        {
            for (int i =1; i <=columns;i++ )
            {
            matrix[index(i,j)] = arr(i,j);  //This is the assigning part, the loop above loops everything so that each term is assigned.
            }
        }
    }
    return *this;   //Return
  }

  istream & operator >> (istream &is, dymatrix &om)   //Overloading ">>" operator here to
  {
    cout<<"Please enter the number of rows you want"<<endl;
    is >> om.rows;  //Inputting number of rows.
    cout<<"Enter the number of columns you want"<<endl;
    is >> om.columns;   //Inputting number of columns.
    cout<<"Enter matrix"<<endl;
    om.matrix = new double [om.rows*om.columns];    //Making a dynamic array here to put the data in.
    for (int j = 1; j <= om.rows; j++)
    {
        for (int i = 1; i <= om.columns; i++)
        {
            is >> om.matrix[om.index(i,j)]; //Putting in the data into this dynamic array for each element.
        }
    }
    return is;
  }
  ostream & operator << (ostream &os,   dymatrix &om)  //To output the matrix in an standard matrix way
  {
    for(int j= 1; j<=om.rows; j++)
    {
        os<<endl<<endl;
        for (int i = 1; i <=om.columns;i++)
        {
            os << om.matrix[om.index(i,j)]<<"\t";   //Similar method used in istream.
        }
    }
    return os;
  }
int main()
{
  dymatrix a1;
  cin >> a1;  //Define the rows of the matrix
  cout << a1<<endl<<endl;
  dymatrix a2;
  cin >> a2;
  cout << a2<<endl<<endl;
  dymatrix resu_a1;
  resu_a1=a1+a2;
  cout<<"Addition = "<<resu_a1<<endl;
  dymatrix resu_a3;
  resu_a3 = a1;
  cout<<"Assigning = "<<resu_a3<<endl;
  return 0;
}  

This is my destructor:

~dymatrix(){cout<<"Destructor called"<<endl;delete[] matrix;}  

This is when I overload the assignment "=" operator(outside the class) which seems to work:

dymatrix & dymatrix::operator = (dymatrix &arr) //Overloading "=" operator.
  {
    if (&arr == this) return *this; //If the array is the same, no need to change, just to print. The key word "this" is a pointer to the object, and *this gives the object.
    delete[] matrix; matrix =0; rows =0; columns =0;
    rows = arr.rows;   //Setting row length.
    columns = arr.columns;    //Setting column length.
    if(rows*columns > 0)
    {
        matrix = new double [rows*columns]; //Defining a dynamic array here.
        for (int j = 1; j <= rows; j++) //Assigning each term to each term.
        {
            for (int i =1; i <=columns;i++ )
            {
            matrix[index(i,j)] = arr(i,j);  //This is the assigning part, the loop above loops everything so that each term is assigned.
            }
        }
    }
    return *this;   //Return
  }

But when I overload the addition operator "+" inside the class, I get the error which I quoted above.

dymatrix operator + (dymatrix &arr)  //overloading addition.
{   

    if (rows !=arr.rows && columns != arr.columns)
    {
        cerr<<"SIZE DO NOT MATCH, YOU FAIL"<<endl; exit(1);
    }
    dymatrix new_matrix(rows,columns);
    for (int j = 0; j < arr.rows*arr.columns; j++)
    {
        //for (int i = 1; i <= arr.columns; i++)
        //{
        //cout<<"****"<<j<<endl;    
        new_matrix.matrix[j]= matrix[j]+arr.matrix[j]; //Putting in the data into this dynamic array for each element.
        //}
    }
    return new_matrix;
}

This is my int main:

int main()
{
  dymatrix a1;
  cin >> a1;  //Define the rows of the matrix
  cout << a1<<endl<<endl;
  dymatrix a2;
  cin >> a2;
  cout << a2<<endl<<endl;
  dymatrix resu_a1;
  resu_a1=a1+a2;
  cout<<"Addition = "<<resu_a1<<endl;
  dymatrix resu_a3;
  resu_a3 = a1;
  cout<<"Assigning = "<<resu_a3<<endl;
return 0;
} 
Xiyang Liu
  • 81
  • 5
  • `dymatrix & dymatrix::operator = (dymatrix &arr)` function parameter should be const reference (or by value if performing copy and swap idiom) – Neil Kirk Mar 06 '15 at 01:43

1 Answers1

0

You are returning copies of your matrix when you call operator +, but your dymatrix lacks a user-defined copy constructor:

dymatrix(const dymatrix&);  // this is the missing function

You must implement this function to have a return-by-value of your dymatrix to be correct. Without this function, your code will exhibit undefined behavior, and will more than likely crash when it comes time to object destruction (double deletion errors and/or memory leaks).

Look up the "Rule of 3". If you implement a user-defined assignment operator, then a user-defined copy constructor and destructor should also be implemented:

What is The Rule of Three?

The easiest way to do this is to move most (if not all) of that code from your operator=, to the copy constructor. Then implement your operator= in terms of the copy constructor using the copy/swap idiom:

What is the copy-and-swap idiom?

As to your implementation of operator=: why are you doing all of that work to copy from one array to another? It should be a straight loop, going from 0 to rows*columns. You shouldn't need to call all of those extraneous functions to get the index and so forth.

But going back to the copy constructor, it would more than likely look like this:

#include <algorithm>
//...
dymatrix::dymatrix(const dymatrix& arr) : 
        rows(arr.rows), columns(arr.columns),
        matrix(new double[arr.rows * arr.columns])
{
   std::copy(arr.matrix, arr.matrix + rows*columns, matrix);
}

Now your assignment operator can be coded like this:

#include <algorithm>
//...
class dymatrix {
   //...
    dymatrix& operator=(dymatrix arr); // use this version of assignment operator
  //...
};

//...
dymatrix& dymatrix::operator=(dymatrix arr)  
{
   std::swap(rows, arr.rows);
   std::swap(columns, arr.columns);
   std::swap(matrix, arr.matrix);
   return *this;
}

The other point is that you should have implemented operator+= before you implemented operator +. The reason is that you could then write operator + in terms of operator +=, thus creating two operators, with operator + being essentially a 3 line function (the code in your current operator + would be moved to the operator +=.


Another point is that your operator+ should pass a const dymatrix&, instead of dymatrix&. You are not changing the parameter within the function, so it should be passed as const reference.


The last point is that if the matrices are not the same size when you add them, your code should throw an exception -- it should never just exit the program as your code is doing, and especially not within a class.

This should explain why you should never do this (I know it is probably a school exercise, but it is such a poor thing to do):

exit() call inside a function which should return a reference

Community
  • 1
  • 1
PaulMcKenzie
  • 34,698
  • 4
  • 24
  • 45
  • Thank you so much, this got rid of the error message.The only problem left is that the addition of 2 matrices gave me a memory location. I did do a cout inside the "addition" overloading function to check whether if it was returning a correct value which it did, but the cout in my main function gave me a memory location :s – Xiyang Liu Mar 06 '15 at 01:56
  • I edited my answer to correct the call to `copy` in the copy constructor. Your code should output correctly now. – PaulMcKenzie Mar 06 '15 at 02:16