overloading = operator in a class for a dynamic array C++

Question

I have been trying to add two 2D arrays. C++ start crashing once after I added the overload function for "=". The compiler just crashes. However without adding

cout << "combining the two is" << endl;
example3 = example1 + example2;
example3.outPut();

and disabling overload operators member functions, everything works fine.
Could you please help?

enter code here

#include <iostream>
using namespace std;
#include <string>
#include <fstream>
#include <cmath>
#include <cstdlib>
#include <ctime>
#include <vector>
#include <windows.h>
#include <cstring>
#include <cctype>
#include <iomanip>
#include <algorithm>
#include<sstream>


class TwoD
{
private:
int MaxRows;
int MaxCols;
double** outerArray;

public:
TwoD(int maxRows, int maxCols)
{
    MaxRows = maxRows;
    MaxCols = maxCols;
    outerArray = new double *[MaxRows];
    for (int i = 0; i < MaxRows; i++)
        outerArray[i] = new double[MaxCols];
}

void input()
{
    for (int k = 0; k < MaxRows; k++)
        for (int j = 0; j < MaxCols; j++)
            cin >> outerArray[k][j];
}

void outPut()
{
    for (int l = 0; l < MaxRows; l++)
    {
        for (int m = 0; m < MaxCols; m++)
            cout << outerArray[l][m] << " ";
        cout << endl;
    }
}

const TwoD operator = (const TwoD& rightSide)
{
    for (int l = 0; l < MaxRows; l++)
    {
        for (int m = 0; m < MaxCols; m++)
            outerArray[l][m] = rightSide.outerArray[l][m];
        cout << endl;
    }

    return *this;
}



const TwoD operator + (const TwoD& rightSide)
{
    for (int l = 0; l < MaxRows; l++)
    {
        for (int m = 0; m < MaxCols; m++)
            outerArray[l][m] = outerArray[l][m] + rightSide.outerArray[l][m];
        cout << endl;
    }

    return *this;
}

~TwoD()
{
    for (int i = 0; i < MaxRows; i++)
        delete[] outerArray[i];
    delete[] outerArray;
}

};

int main()
{
TwoD example1(3, 3), example2(3,3), example3(3,3);
cout << "input example1" << endl;
example1.input();
example1.outPut();

cout << "input example2" << endl;
example2.input();
example2.outPut();

cout << "combining the two is" << endl;
example3 = example1 + example2;
example3.outPut();


return 0;
}

could you be more specific? Is there anything wrong with.....const TwoD operator = (const TwoD& rightSide) — Ujae Kang, Sep 09 '13 at 10:50
operator = should not return by value. This creates a pointless copy. Return by non-const reference is the standard practice. You also need a copy constructor. — Neil Kirk, Sep 09 '13 at 13:17

score 3 · Answer 1 · edited May 23 '17 at 11:50

For a class/struct which dynamically allocates memory, you need to follow rule of three.

class TwoD
{
public:
   TwoD(const TwoD& other);             // copy constructor
   TwoD& operator = (const TwoD& other) // copy assign operator
};

I'd suggest you use a vector of vector to represent 2D array, you don't even need to write copy construct, memory management etc.

#include <vector>
struct TwoD
{
    std::vector<std::vector<double> > outerarray;
};

score 3 · Accepted Answer · edited May 23 '17 at 12:05

The compiler just crashes.

Really? For me, the program compiles, but crashes at runtime due to a double deletion.

Could you please help?

To make the class correctly copyable:

You need both a copy constructor and a copy-assignment operator, per the Rule of Three. You only have the assignment operator.
The assignment operator should return a reference, not a copy.

A suitable copy constructor might look like

TwoD(const TwoD& other) : TwoD(other.MaxRows, other.MaxCols)
{
    for (int l = 0; l < MaxRows; l++)
        for (int m = 0; m < MaxCols; m++)
            outerArray[l][m] = other.outerArray[l][m];    
}

(If you can't use C++11's delegating constructors, then you'll need to either duplicate the default constructor's initialisation, or move it out into a separate function called by both constructors).

Once you have a copy constructor, you might consider using the copy-and-swap idiom to get a more exception-friendly assignment operator. In C++11, you might also consider making the class movable (by copying the pointer to the target, and nullifying the one in the victim), to avoid unnecessary memory allocation in some situations.

Also, operator+ shouldn't modify its argument; it should return a freshly created object. Your implementation is more suitable for operator+= (although, like operator=, that should return a reference, not a copy).

Once you've learned how manual memory management works, you should use classes like the standard containers and smart pointers to solve all these tedious problems for you. In particular, std::vector is a dynamic array.

Thanks for your instructions. I think I learned what I need to work on. What you explained make sense and I need to do some more research to solidify my understanding. I want to use vector another day and better understand "copy constructor" for now. – — Ujae Kang, Sep 09 '13 at 11:17
Mike, I am still struggling. I had to leave for work yesterday, I thought your comment would fix it. But it didn't. Could you please take a look at my question ....http://stackoverflow.com/questions/18738504/copy-constructor-big-3-c-issue — Ujae Kang, Sep 11 '13 at 10:40

score 1 · Answer 3 · edited Sep 11 '13 at 12:14

1

Prev answer was wrong, let me take another shot at it.

When you create copies of the object, your matrix is not getting copied over, only the pointer to it is. This means that you have multiple objects, particularly here the temporary objects, which when they get destructed, will delete the matrix, and then at the end of the function, you will delete them again. This is your crash, a double free.

You need to Define a "Copy Constructor" which would make a true copy of your object in a well mannered way, which is what Billz is refering to, or take the easy route and use C++ library classes like vector which take care of this for you.

int MaxRows;
int MaxCols;
double** outerArray;

would become

std::vector<std::vector<double> > matrix = new std::vector(MaxRows, new std::vector(MaxCols, 0.0))

You can read more about vector in google or at http://en.cppreference.com/w/cpp/container/vector

edited Sep 11 '13 at 12:14

answered Sep 09 '13 at 10:51

Karthik T

31,456
5
68
87

I must not be understanding "return *this;" part very well. I am correct? Sorry I am fairly new to C++ and could not find any online answer for this. – Ujae Kang Sep 09 '13 at 10:53
`operator=` will not call itself if it returns by value, it will call the copy constructor. There will be no stack overflow. – interjay Sep 09 '13 at 10:54
@interjay yup you are right.. and that is the problem, no user defined copy constructor.. – Karthik T Sep 09 '13 at 10:58
Thanks for your instructions. I think I learned what I need to work on. What you explained make sense and I need to do some more research to solidify my understanding. I want to use vector another day and better understand "copy constructor" for now. – Ujae Kang Sep 09 '13 at 11:10
@UjaeKang Glad we could help you! – Karthik T Sep 10 '13 at 03:41

overloading = operator in a class for a dynamic array C++

3 Answers3