How to overload operator= in a templated class with a dynamically sized array

Question

I'm new to making my own template classes in C++, and after several hours of searching online for answers and toying with the function & its parameters, I've given up. I'm having run-time trouble with the following class' "=" operator:

In matrix.h:

template <class datatype> class Matrix{
  datatype** element;
  unsigned int m,n;

  public:

  Matrix(unsigned int M, unsigned int N,datatype x){
    m=M;    // # of rows
    n=N;    // # of cols
    element=new datatype*[m];
    for(int i=0;i<m;i++) element[i]=new datatype[n];
    for(int i=0;i<m;i++)
      for(int j=0;j<n;j++)
        element[i][j]=x;
  }

  void print(){
    for(int i=0;i<m;i++){
      for(int j=0;j<n;j++) cout<<element[i][j]<<" ";
      cout<<"\n";
    }
  }

  Matrix operator=(Matrix A){
    for(int i=0;i<m;i++) delete[] element[i];
    delete[] element;
    m=A.m;
    n=A.n;
    element=new datatype*[m];
    for(int i=0;i<m;i++) element[i]=new datatype[n];
    for(int i=0;i<m;i++)
      for(int j=0;j<n;j++)
        element[i][j]=A.element[i][j];
    return *this;
  }
};

When I go to test this, compilation & linking run smoothly w/o error, and I get a perfectly valid print. But when trying to assign one matrix to the value of another, the program crashes w/ message "matrix_test has stopped working". Here's my testing code, in matrix_test.cpp:

Matrix<int> M(5u,3u,0);
Matrix<int> P(2u,7u,3);

int main(){
    M.print();
    cout<<"\n";
    P.print();
    cout<<"\n";
    P=M;
    P.print();        
}

Thanks in advance for the help!

Your assignment operator signature should look like `Matrix& operator=(const Matrix& A)`. You also need a _copy constructor_ and _destructor_. See: [What is The Rule of Three?](http://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) — Blastfurnace, Oct 25 '13 at 01:34
@Blastfurnace: Actually, I think the signature is absolutely the correct one! However, the implementation is not! ;-) — Dietmar Kühl, Oct 25 '13 at 01:52
@DietmarKühl: I could agree with using `Matrix& operator=(Matrix A)` instead. — Blastfurnace, Oct 25 '13 at 02:06
@Blastfurnace: OK, I didn't notice that the `Matrix` was also returned by value: that is, indeed, a Bad Idea. Passing the argument by value is quite reasonable as a copy is needed anyway (see my answer). — Dietmar Kühl, Oct 25 '13 at 02:08
@Dietmar Kühl: How would i change the implementation then? I did already have a destructor and a copy constructor, I just didnt bother to show them here, thinking they wouldnt affect the answer to my question. What's wrong with the implementation though? Thanks again for the help! — A Frayed Knot, Oct 25 '13 at 02:09
@KyleMcCormick: Dietmar's solution is a very good idea. For completeness I'll link to a related SO question: [What is the copy-and-swap idiom?](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom) — Blastfurnace, Oct 25 '13 at 02:13

score 1 · Accepted Answer · answered Oct 25 '13 at 02:07

First off, the implementation of your copy-assignment is flawed in a rather fundamental way: When you delete[] the representation and then allocate a new copy, the allocation may throw in which case your original matrix is delete[]d and can't be recovered. Thus, the assignment isn't exception safe.

The best implementation of the copy-assignment operator is to leverage the copy-construction and the swap() member. Of course, both of these members are missing from your class but let's get to this later:

Matrix& Matrix::operator= (Matrix other) {
    other.swap(*this);
    return *this;
}

When passing an argument by value, it is actually getting copied. To copy the object, you'll need a copy constructor. In general, if you need a copy-assignment you typically also need a copy constructor and a destructor (there are cases when you only need a copy-assignment to make the copy-assignment strongly exception-safe but that's a different discussion).

The purpose of the copy constructor is to copy another object, e.g., when the object is passed by value:

Matrix::Matrix(Matrix const& other)
    : element(new datatype*[other.m])
    , m(other.m)
    , n(other.n)
{
    int count(0);
    try {
        for (; count != m; ++count) {
            this->element[count] = new datatype[m];
            std::copy(other.element[count], other.element[count] + m,
                      this->element[count]);
        }
    }
    catch (...) {
        while (count--) {
            delete[] this->element[count];
        }
        delete[] this->element;
        throw;
    }
}

I'm not sure if the recovery from an exception is actually correct: I can't deal with the complexity of coping with all these pointers! In my code I would make sure that all resources immediately constructor an object dedicated to releasing them automatically but that would require to change the type of the objects. Given the definition of the type, a destructor is also needed:

Matrix::~Matrix() {
    for (int count(this->m); count--; ) {
        delete[] this->element[count];
    }
    delete[] this->element;
}

Finally, for larger objects a swap() member is often handy. The purpose of swap() is to just exchange the content of two objects. The way to implement it is to do a memberwise std::swap():

void Matrix::swap(Matrix& other) {
    using std::swap;
    swap(this->element, other.element);
    swap(this->n, other.n);
    swap(this->m, other.m);
}

Given that all members of this class are built-in types (although they probably shouldn't be), the using-dance isn't really needed. However, if there are specialized overloads of swap() in other namespaces than std::swap() for user-defined types, the above approach makes sure these are found by argument dependent look-up.

Thank you! This definitely helped. I hadn't considered using std::swap on an object passed in by value. I was actually using exception handling at first, but since I was declaring (very) small arrays to test it with, I just wanted to make sure the fundamental ops worked first. — A Frayed Knot, Oct 25 '13 at 02:19

How to overload operator= in a templated class with a dynamically sized array

1 Answers1