Matrix Class and operator= overloading

Question

I am facing a little problem that I can't solve on my own. I am writing a program which implements very simple operations on matrixes. The problem is that when I am trying to do something like this:

Matrix first(5);
Matrix e;

first.identityMatrix();
e = first;
cout << first;
cout << e;

Short explanation: I want to assign square matrix to a matrix without dimensions.

Second cout doesn't show anything. But when I change Matrix e() to Matrix e(5), everything works perfectly. I know that the bug resides in this piece of code:

Matrix& Matrix::operator=(const Matrix& tmp)
{
if (this->a==0 && this->b == 0)
{
    this->matrix = new double*[tmp.a];

    for (int i=0;i<tmp.a;i++)
        this->matrix[i] = new double[tmp.b];
} else {
    try {
        if (this->a!=tmp.a || this->b!=tmp.b)
            throw wrongSize();
    } catch (wrongSize& e) {
        e.message();
        throw;
    }
}

for (int i=0;i<tmp.a;i++)
{
    for (int j=0;j<tmp.b;j++)
    {
        this->matrix[i][j] = tmp.matrix[i][j];
    }
}

return *this;
}

After some attempts I guess that something is wrong with memory allocation, but I am not sure. To me it ought to work correctly due to the fact that I return the reference to the current object. I think that constructors might be useful as well:

Matrix::Matrix()
{
a = 0;
b = 0;
matrix = NULL;
}

Matrix::Matrix(int a)
{
try {
    if (a==0)
        throw wrongRowOrColNumber();
} catch (wrongRowOrColNumber& e) {
    e.message();
    throw;
}
this->a = a;
this->b = a;
this->matrix = new double*[a];

for (int i=0;i<a;i++)
    matrix[i] = new double[a];
for (int i=0;i<a;i++)
    for (int j=0;j<a;j++)
        matrix[i][j] = 0;
}

Matrix::Matrix(int a, int b)
{
try {
    if (a==0 || b==0)
        throw wrongRowOrColNumber();
} catch (wrongRowOrColNumber& e) {
    e.message();
    throw;
}

if (a==b)
{
    try {
        if (a==0)
            throw wrongRowOrColNumber();
    } catch (wrongRowOrColNumber& e) {
        e.message();
        throw;
    }
    this->a = a;
    this->b = a;
    this->matrix = new double*[a];

    for (int i=0;i<a;i++)
        matrix[i] = new double[a];
    for (int i=0;i<a;i++)
        for (int j=0;j<a;j++)
            matrix[i][j] = 0;
} else {
    this->a = a;
    this->b = b;
    this->matrix = new double*[a];

    for (int i=0;i<a;i++)
        matrix[i] = new double[b];
    for (int i=0;i<a;i++)
        for (int j=0;j<b;j++)
            matrix[i][j] = 0;
}
}

Operator <<:

friend ostream& operator<<(ostream& buffer, const Matrix& tmp)
{
    for (int i=0;i<tmp.a;i++)
    {
        for (int j=0;j<tmp.b;j++)
        {
            buffer << tmp.matrix[i][j] << " ";
        }
        buffer << endl;
    }
    return buffer;
};

IdentityMatrix:

Matrix& Matrix::identityMatrix()
{
try {
    if (this->a!=this->b)
    {
        throw wrongSize();
    }
} catch (wrongSize& e) {
    e.message();
    throw wrongSize();
}
int row = this->a;

for (int i=0;i<row;i++)
{
    for (int j=0;j<row;j++)
    {
        if (i==j)
            this->matrix[i][j] = 1;
        else
            this->matrix[i][j] = 0;
    }
}

return *this;
}

Why are you doing allocation like that? Do you realize you have a leak if one of the `new`s throws? — Pubby, Nov 23 '12 at 21:52
Use [copy and swap](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). — Alexandre C., Nov 23 '12 at 21:55
Can't see any obvious reason for the failure. Can we perhaps see identityMatrix and operator<<. — john, Nov 23 '12 at 21:58
I tried to call Matrix(a) but it doesn't work, the program crashes then. — mic4ael, Nov 23 '12 at 21:59

Olaf Dietsche · Accepted Answer · 2012-11-23T22:08:37.867

You throw an exception several times and catch it immediately after, just to show a message and rethrow. You can save the try/catch, if you just show the message and then throw the exception instead.

In your assignment operator, you must copy the dimensions a and b as well.

Matrix& Matrix::operator=(const Matrix& tmp)
{
    if (this->a==0 && this->b == 0)
    {
        this->a = tmp.a;
        this->b = tmp.b;

        this->matrix = new double*[tmp.a];
        ...
    }
    ...
}

In your constructor Matrix::Matrix(int a, int b), you have an if (a == b) ... else. You can remove the if part and just leave the else part. This way, you have less code and less potential for bugs.

So obvious but only when you've seen it. – john Nov 23 '12 at 22:06 — john, Nov 23 '12 at 22:06

Matrix Class and operator= overloading

1 Answers1