CODEWITHSUNDEEP
c++overloadingdestructorfree
SarahHime
SarahHime

Reputation: 43

C++ Operator overload calling destructor

For my first try with operator overloading I created a vector class and tried to sum up two vectors. My class contains an array and a vector of int that both contain the same elements.

Addition works fine with the std::vector but I encounter two issues with the array. It seems that the destructor is called at the end of the summing operation which produces a "double free or corruption" error (core dump). Plus, the first two elements of the array are always equal to zero.

Should I also overload the delete or am I missing a thing ?

The header file:

#ifndef MYVECTOR_INCLUDE
#define MYVECTOR_INCLUDE
#include <iostream>
#include <stdexcept>
#include <cstring>
#include <vector>

class MyVector {
public:
    MyVector(int n);
    ~MyVector();
    void set(int idx, int value);
    int get(int idx);
    void print();    
    MyVector &operator=(const MyVector &v);
    MyVector operator+(const MyVector &v);
private:
    int *data;
    std::vector<int> data_vector;
    int size1;
    int size2;
};
#endif

The cpp file:

#include "../include/myvector.hpp"

MyVector::MyVector(int n) {
    data = new int [n];
    data_vector.resize(n);
    size1 = n;
    size2 = 1;
}
MyVector::~MyVector() {
    if (data != NULL) {
        delete [] data;
    }
}
void MyVector::set(int idx, int value) {
    data_vector[idx] = value;
    data[idx] = value;
}
int MyVector::get(int idx) {
    return data_vector[idx];
}
void MyVector::print() {
    std::cout << "Vector data" << std::endl;
    for (int i = 0; i < size1; ++i) {
        std::cout << data_vector[i] << " ";
    }
    std::cout << std::endl;
    std::cout << "Data" << std::endl;
    for (int i = 0; i < size1; ++i) {
        std::cout << data[i] << " ";
    }
    std::cout << std::endl;
}
MyVector &MyVector::operator=(const MyVector &v) {
    if (this == &v)
        return *this;
    size1 = v.size1;
    size2 = v.size2;
    std::copy(v.data_vector.begin(), v.data_vector.end(), data_vector.begin());
    memcpy(&data, v.data, sizeof(int)*size1);
    return *this;
}
MyVector MyVector::operator+(const MyVector &v) {
    if ((size1 == v.size1) && (size2 == v.size2)) {
        MyVector res = MyVector(size1);
        for (int i = 0; i < size1; ++i) {
            res.data_vector[i] = data_vector[i] + v.data_vector[i];
            res.data[i] = data[i] + v.data[i];
        }
        return res;
    }
    else
        throw std::length_error("Vector dimensions must agree.");
}

Thank you.

Upvotes: 2

Views: 2242

Answers (1)

Fran&#231;ois Andrieux
Fran&#231;ois Andrieux

Reputation: 29023

See the rule of three/five/zero.

The rule of three states

If a class requires a user-defined destructor, a user-defined copy constructor, or a user-defined copy assignment operator, it almost certainly requires all three.

In this case, you provided the destructor and copy assignment operator but forgot the copy constructor.

In operator+ you create a local object MyVector res which is later copied out and then destroyed. Since you haven't implemented a copy constructor, the default copy constructor will simply copy the original data pointer from res to another MyVector. When res is destroyed, the array pointed to by it's data member will be deleted, leaving the copied vector's data pointing to a deleted object.

Upvotes: 6

Related Questions