0

I've made a class that stores a pointer to a dynamic 2D array, I've overloaded the '+' operator so that it returns the addition of two matrices:

class matrix
{
    int * mat, row, col;
public:
    matrix(int r, int c)
    {
        row = r;
        col = c;
        mat = new int[row * col];
    }

    matrix operator + (matrix m)
    {
        matrix result(row, col);
        for (int i = 0; i < row; i++)
        {
            for (int j = 0; j < col; j++)
            {
                result.mat[i * result.col + j] = mat[i * col + j] + m.mat[i * m.col + j]
            }
        }
        //result.display(); //result contains expected values i.e. added values
        return result;
    }

    matrix(matrix &m)
    {
      this->mat = m.mat;
      this->row = row;
      this->col = col;
    }


  ~matrix()
   {
     delete[] mat;
   }

  void operator = (matrix m)
{
    if (row != m.row || col != m.col)
    {
        cerr << "Incompatible Matrix Assignment";
        return;
    }

    else
    {
        for(int i = 0; i < row; i++)
        {
            for(int j = 0; j <col; j++)
            {
                mat[i * col + j] = m.mat[i * m.col + j];
            }
        }
    }

    return;
} 

};

int main()
{
    matrix m1(2, 2); //m1.input(); //input has been taken
    matrix m2(2, 2); //m2.input(); //input has been taken
    matrix m3(2, 2);
    m3 = m1 + m2;
    //m3.output;
}

'+' function in class returns a local matrix variable 'result' it contains expected values but when result is returned by the function only first two values of mat(data member of matrix class) contains garbage i.e. m[0][0] and m[0][1], rest of the array variables have expected values. Make array size 3X3 or 4X4 it doesn't create a difference only first two variables contain garbage.

Nilesh Kumar
  • 343
  • 1
  • 5
  • 18
  • 1
    Where's the destructor? And please look up the [rule of three](http://en.cppreference.com/w/cpp/language/rule_of_three). – rustyx Mar 10 '18 at 14:58
  • 4
    I bet you do `delete mat;` in the destructor, don't you? If so, then put a breakpoint in the destructor, debug your code and see what happens. – Rabbid76 Mar 10 '18 at 15:00
  • yeah I've declared a destructor which executes 'delete[] mat', just forgot to mention it here. – Nilesh Kumar Mar 12 '18 at 02:06
  • I've used vectors for this before but I was wondering what is the problem with int *. Please suggest a thing for this. – Nilesh Kumar Mar 12 '18 at 02:10
  • @Rabbid76 ,I bet you do delete mat; in the destructor, don't you? If so, then put a breakpoint in the destructor, debug your code and see what happens. what is meant by the breakpoint in destructor, please explain. And I've updated the above code with destructor please check that and please suggest what has gone haywire – Nilesh Kumar Mar 12 '18 at 02:10
  • @user9391556 You can read [StackOverflow: How to debug using GDB](https://stackoverflow.com/questions/2069367/how-to-debug-using-gdb) to understand what is a "breakpoint". Or read the docs of your debugger. – user202729 Mar 12 '18 at 02:15
  • 1
    Look up the "rule of three" as a starting point. If a destructor needs to clean up a resource (memory) in your case, it is also typically necessary to define both a copy constructor and an assignment operator. Otherwise, the destructors for two distinct objects, one created as a copy of the other, can attempt to release the SAME resource, and give undefined behaviour. Under C++11 and later, the rule of 3 becomes either the rule of 5 or rule of zero. – Peter Mar 12 '18 at 02:26
  • @Peter, so you're suggesting that I should define a copy ctor, but as far as I know 'm3' above is already initialized I'm simply assigning the values which won't invoke the copy ctor, it'll simply copy the values member by member. By the way thanks for the suggestion, will update if it works. – Nilesh Kumar Mar 12 '18 at 02:57
  • @Nilesh - You need to implement a copy constructor AND an `operator=()`. Your `operator+()` returns an object by value - which is correct. When one of the members being copied by value is a pointer to dynamically allocated memory (`mat` in your case) then copying it by value means that TWO objects end up with a pointer to the SAME memory (so their destructors will both release it -> undefined behaviour). Implementing a copy constructor - to dynamically allocate the memory for a copy corrects that. – Peter Mar 12 '18 at 03:29
  • @Peter as you said I already overloded = operator but then this problem occurred so I removed `operator = ()`, I thought that function might be buggy. Now when I declare `operator = ()` again with copy constructor. It gives some errors. please check them I've edited the code above. Note that I've made class matrix as a template class just didn't mentioned it here. – Nilesh Kumar Mar 13 '18 at 04:46
  • The problem is that your definition of the copy constructor does not behave as required. After it is invoked, the object being created (`*this`) and `m` both contain the same pointer - which is the same problem as if you don't provide a copy constructor. Also, it is preferable that both copy constructor and `operator=()` accept their parameter by `const` reference. – Peter Mar 13 '18 at 09:03

1 Answers1

1

Follow the rule of 0/3/5. In this case the rule of 0 is best.

Of these 5 special member functions:

  • dtor

  • copy assign/ctor

  • move assign/ctor

implement manually 0, 3 or 5 of them (either dtor and a assign/ctor pair, all 5, or none). Sometimes =delete can replace manually implementing them. If you implement a dtor and fail to do anything with the move/copy ctors, your code will probably be broken.

By far the easiest is to follow the rule of zero.

std::vector<int> mat;

then instead of mat = new int[row * col];:

mat.resize(row * col);

finally don't write a dtor (destructor).

By having a resource management class (vector) manage your resources, you can make your business logic class (matrix) not be full of error prone resource management code.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • sorry, but I've used vectors for this purpose I was just wondering why it is losing data, and just for the first two variables. – Nilesh Kumar Mar 12 '18 at 02:07
  • Can you please explain what is assign/ctor thing I haven't heard of this – Nilesh Kumar Mar 12 '18 at 02:17
  • 1
    @user `operator=` is assign. Ctor means constructor. Copy means `Matrix const&`. Move means `Matrix&&`. See [Rule of Three](https://en.m.wikipedia.org/wiki/Rule_of_three_(C%2B%2B_programming)) – Yakk - Adam Nevraumont Mar 12 '18 at 02:23