Search code examples
c++operator-overloadingdestructordynamic-arrays

assignment overloading explicitly calls destructor(After execution of other binary overloaded operators)


My problem is easy( double free or corruption (fasttop): ) and there is a solution for this (here). But, I couldn't implement it. I don't know where is the problem. When I'd traced my program I saw that it happens after binary operators execution... As an example;

arr1 = arr2 + arr3;

After execution this statement it calls destructor. So my destructor will call end of the scope so that it is called more than required. My Class definition is;

class IntArray
{
public:
    IntArray(int size);
    //IntArray(const IntArray& ref);
    ~IntArray();
    IntArray& operator= (const IntArray&);
    const int& operator[] (int index) const;
    friend ostream& operator <<(ostream&, IntArray);
    friend istream& operator >>(istream&, IntArray);
    friend IntArray operator *(const IntArray&, const IntArray&);
    friend IntArray operator +(const IntArray&, const IntArray&);
    friend IntArray operator -(const IntArray&, const IntArray&);
    friend bool operator ==(const IntArray&, const IntArray&);
    friend bool operator <(const IntArray&, const IntArray&);
    friend bool operator >(const IntArray&, const IntArray&);
    const int& getArr(const int index) const{
        return arr[index];
    }
    void setArr(int value, int index){
        arr[index] = value;
    }
private:
    int* arr;
    int sz;
};

And my destructor is;

IntArray::~IntArray(){
    delete[] arr;
}

And one of my binary overloading functions;

 IntArray operator- (const IntArray& refOne, const IntArray& refTwo){
        IntArray newArray(refOne.sz);
        int value;
        for(int i = 0; i < refOne.sz-1; i++){
                value = refOne.getArr(i) - refTwo.getArr(i);
                newArray.setArr(value, i);
            }
        return newArray;
    }

Whether it needs or not, here my constructor;

IntArray::IntArray(int size){
    sz = size;
    arr = new int[size];
    for (int i = 0;i < size; i++)
        arr[i] = 0;
}

I didn't use copy constructor. Instead I used assignment operator. And my all code is;

/*
 * sampleArrayImplemtation.cpp
 *
 *  Created on: Jul 31, 2012
 *      Author: musbuntu
 */

#include <iostream>
#include <cstdlib>
using namespace std;
class IntArray
{
public:
    IntArray(int size);
    //IntArray(const IntArray& ref);
    ~IntArray();
    IntArray& operator= (const IntArray&);
    const int& operator[] (int index) const;
    friend ostream& operator <<(ostream&, IntArray);
    friend istream& operator >>(istream&, IntArray);
    friend IntArray operator *(const IntArray&, const IntArray&);
    friend IntArray operator +(const IntArray&, const IntArray&);
    friend IntArray operator -(const IntArray&, const IntArray&);
    friend bool operator ==(const IntArray&, const IntArray&);
    friend bool operator <(const IntArray&, const IntArray&);
    friend bool operator >(const IntArray&, const IntArray&);
    const int& getArr(const int index) const{
        return arr[index];
    }
    void setArr(int value, int index){
        arr[index] = value;
    }
private:
    int* arr;
    int sz;
};
IntArray& IntArray::operator= (const IntArray& ref){
    if(this!=&ref){
        delete[] arr;
        sz = ref.sz;
        arr = new int[sz];
        for (int i=0; i<sz;i++)
            arr[i] = ref.arr[i];
    }
    return *this;
}
bool operator< (const IntArray& valOne, const IntArray& valTwo){
    int flag(0);
    for(int i = 0;i<valOne.sz;i++)
    {
        if (valOne[i] < valTwo[i]){
            flag = 1;
        }
        else{
            return(0);
        }
    }
    return(flag);
}
bool operator> (const IntArray& valOne, const IntArray& valTwo){
    int flag(0);
    for(int i = 0;i<valOne.sz;i++)
    {
        if (valOne[i] > valTwo[i]){
            flag = 1;
        }
        else{
            return(0);
        }
    }
    return(flag);
}

bool operator== (const IntArray& valOne, const IntArray& valTwo){
    int flag(0);
    for(int i = 0;i<valOne.sz;i++)
    {
        if (valOne[i] == valTwo[i]){
            flag = 1;
        }
        else{
            return(0);
        }
    }
    return(flag);
}
IntArray operator- (const IntArray& refOne, const IntArray& refTwo){
    IntArray newArray(refOne.sz);
    int value;
    for(int i = 0; i < refOne.sz-1; i++){
            value = refOne.getArr(i) - refTwo.getArr(i);
            newArray.setArr(value, i);
        }
    return newArray;
}
IntArray operator+ (const IntArray& refOne, const IntArray& refTwo){
    IntArray newArray(refOne.sz);
    int value;
        for(int i = 0; i < refOne.sz-1; i++){
            value = refOne.getArr(i) + refTwo.getArr(i);
                newArray.setArr(value, i);
            }
    return newArray;
}
IntArray operator* (const IntArray& refOne, const IntArray& refTwo){
    IntArray newArray(refTwo.sz);
    int value;
        for(int i = 0; i < refOne.sz-1; i++){
                value = refOne.getArr(i) * refTwo.getArr(i);
                newArray.setArr(value, i);
            }
    return newArray;
}
istream& operator>> (istream& inputStream, IntArray ref){
    cout << "Enter one by one with respect to a comma> ";
    for(int i = 0; i < ref.sz; i++){
        inputStream >> ref.arr[i];
    }
    return inputStream;
}
ostream& operator<< (ostream& outStream, IntArray ref){
    outStream << "\nYour array has size = " << ref.sz << " and all members are:" << endl;
    for (int i = 0; i < ref.sz; i++){
        outStream << ref[i] << "\t";
    }
    return outStream;
}
const int& IntArray::operator[] (int index) const{
    if (index >= 0 && index < sz){
        return(arr[index]);
    }
    else{
        cerr << "Index out of range, therefore use vectors instead." << endl;
    }
//  return(arr[1]);
}
IntArray::IntArray(int size){
    sz = size;
    arr = new int[size];
    for (int i = 0;i < size; i++)
        arr[i] = 0;
}
//IntArray::IntArray(const IntArray& ref){
//  arr = new int[ref.sz];
//  for(int i=0;i<ref.sz;i++)
//      arr[i] = ref.arr[i];
//}
IntArray::~IntArray(){
    delete[] arr;
}

int main(){
    int sz;
    cout << "Enter size of array> ";
    cin >> sz;
    IntArray arr1(sz);
    arr1.setArr(5, 3);
    IntArray arr2(sz);
    //arr2 = arr1;
    IntArray arr3(sz), arr4(sz), arr5(sz), arr6(sz);
    arr2.setArr(2, 1);
    arr6 = arr1;
    cout << arr6;
    arr3 = arr2 - arr1;
    arr4 = arr1 * arr2;
    arr5 = arr2 + arr1;
    if (arr6 == arr5)
        cout << "It's working.";
    if (arr2 < arr5)
        cout << "It's working, too";
    if (arr4 > arr2)
        cout << "It is also working.";
    cout << arr1 << arr2 << arr3 << arr4 << arr5 << arr6;
    return 0;
}

In order to cope with this problem I had to use deep copy; And to do this; ( According to wikipedia article: ) First add this library/header:

#include <algorithm>

And a deep copy constructor:

IntArray(const IntArray& ref):sz(ref.sz), arr(new int[ref.sz]){
    copy(ref.arr, ref.arr + ref.sz, arr);
}

Thanks for your patience and advices...


Solution

  • Respect the rule of three. All your binary operations are wrong otherwise:

    IntArray operator- (const IntArray& refOne, const IntArray& refTwo){
        IntArray newArray(refOne.sz);
        int value;
        for(int i = 0; i < refOne.sz-1; i++){
                value = refOne.getArr(i) - refTwo.getArr(i);
                newArray.setArr(value, i);
            }
        return newArray;
    }
    

    You return a IntArray by value, a copy is created, and the compiler-generated copy constructor is used. If you want this to behave correctly, you have to implement a copy constructor.

    If instead of commenting it out:

    public:
       IntArray(int size);
       //IntArray(const IntArray& ref);
    

    you could have define it private:

    public:
       IntArray(int size);
    private:
       //IntArray(const IntArray& ref);
    

    and the problem would have been clear from compilation time.