Deep copy of a matrix-like class

Question

I've got a class that shall behave like matrix. So the usecase is something like:

Matrix matrix(10,10);
matrix[0][0]=4;
//set the values for the rest of the matrix
cout<<matrix[1][2]<<endl;

code:

#include <iostream>
#include <cstdlib>
#include <cstdio>
#include <cstring>
#include <sstream>

using namespace std;

class Matrix {
public:


    Matrix(int x, int y);

    class Proxy {
    public:

        Proxy(int* _array) : _array(_array) {
        }

        int &operator[](int index) const {
            return _array[index];
        }
    private:
        int* _array;
    };

   Proxy operator[](int index) const {
      return Proxy(_arrayofarrays[index]);
   }
   Proxy operator[](int index) {
      return Proxy(_arrayofarrays[index]);
   }

    const Matrix& operator=(const Matrix& othersales);

private:
    int** _arrayofarrays;
    int x, y;
};

Matrix::Matrix(int x, int y) {
    _arrayofarrays = new int*[x];
    for (int i = 0; i < x; ++i)
        _arrayofarrays[i] = new int[y];
}

const Matrix& Matrix::operator=(const Matrix& othermatrix) {
    new (this) Matrix(x, y);
    for (int i = 0; i < 3; i++)
        for (int j = 0; j < 3; j++)
            _arrayofarrays[i][j] = othermatrix._arrayofarrays[i][j];

    return *this;
}

int main() {

    Matrix a(2, 3);
    a[0][0] = 1;
    a[0][1] = 2;
    a[0][2] = 3;
    a[1][0] = 4;
    a[1][1] = 5;
    a[1][2] = 6;

    cout << a[1][2] << endl;
    //prints out 6


    const Matrix b = a;
    cout << b[1][2] << endl;

    a[1][2] = 3;

    cout << a[1][2] << endl;
    // prints out 3
    cout << b[1][2] << endl;
    // prints out 3 as well
}

By calling const Matrix b = a; I want to create new instance of Matrix, that will have the same values as a has in that moment. Nevertheless b is being affected by changing the values in a. So if I change some value in a, then it changes in b as well. And I don't want it to behave like this.

So I need to create a copy of b that would not be affected by a itself.

Those might be stupid question, but for me, as a java guy and a C++ newbie are all those stuff really confusing, so thanks for any helpful advices...

Answer 1

There are a few issues with your implementation. The simple one is the error you are getting...

In your Matrix class, operator[] is a non-const member function, which means that it can only be executed on non-const objects. Your operator= takes the right hand side object by const &, and thus you cannot call operator[] on it. The issue here is that you are not offering an implementation of operator[] that promises not to modify the object, once you add that to your type it should compile.

More important than that is the fact that you are leaking memory. When you call operator= on an object you are creating a different Matrix in place, without previously releasing the memory that it held. That is a memory leak.

The implementation of operator= is also not thread-safe. If allocation of memory for any of the internal arrays fails and throws an exception you are leaving your object in a state that is neither the original one nor a valid state. This is bad in itself.

Related to the previous, in as much as correcting one probably leads to the other, your implementation of operator= is not safe if there is aliasing, that is, it fails if you self-assign. The first line will leak the memory and create the new buffers, and from there on you will copy the new buffer into itself, loosing the original information.

Finally, the implementation of the type could be improved if you drop the requirement of using operator[] and use instead operator() with the two indices. User code will have to be adapted (and look less like a bidimensional array) but it provides a bit more freedom of representation (you can store the information internally in any way you want). At the same time, there is no need to allocate an array of pointers and then N arrays of int. You can perform a single memory allocation of NxM ints and do pointer arithmetic to address each location (this is independent of the use of operator[]/operator()), which will reduce the memory footprint and make the layout more compact, improving cache performance (not to mention reducing the number of dynamic allocations by a factor of M)

By calling const Matrix b = a; I want to create new instance of Matrix, that will have the same values of a in that moment. Nevertheless b is being affected by changing the values in a.

Well, this is yet another issue I missed in the first read. The expression const Matrix b = a; does not involve operator=, but rather the copy constructor. Another thing to google: Rule of the Three (basically, if you implement one of copy-constructor, assignment or destructor manually, you probably want to implement all three). Without defining your own copy constructor the compiler will implicitly define one for you that does a shallow copy (i.e. copies the pointers stored in Matrix but does not allocate memory for it). After the copy is made both Matrix share the same memory, and if your destructor releases the memory, you will run into Undefined Behavior when the second destructor runs and tries to delete [] the already deleted memory.