В чем ошибка (работа с памятью, деструкторы)?

Question

[Clean Coder]

Задаю класс - точка в N-мерном пространстве, функции-члены - конструктор, деструктор и сеттер. Также задаю функцию вычисления расстояния между двумя точками Dist. В main'е объявляю 2 точки и вывожу расстояние. Программа работает корректно, но после вывода верного результата зависает. При отсутствии деструктора - все ок. Проблему решило - заменить формальные параметры Dist'a на ссылки, т. е: double Dist(Point& org, Point& dest) вместо double Dist(Point org, Point dest). Но все же до конца не очень понятно, почему не подходит первый вариант.

#include <iostream>
#include <math.h>

using namespace std;

class Point
{
public:
    int _dim;
    double* _coords;
    Point(int N = 3);
    ~Point();
    void Set();
};

Point::Point(int N):
    _dim(N)
{
    _coords = new double[N];
    for(int i = 0; i < N; i++)
        _coords[i] = 0;
}

Point::~Point()
{
    delete _coords;
    _coords = 0;
}

void Point::Set()
{
    cout << "Enter coordinates: ";
    for(int i = 0; i < _dim; ++i)
        cin >> _coords[i];
}

double Dist(Point org, Point dest)
{
    double sumOfSq = 0;
    for(int i = 0; i < org._dim; i++)
            sumOfSq += pow((dest._coords[i] - org._coords[i]), 2);
    return sqrt(sumOfSq);
}

int main()
{
    Point A, B;
    B.Set();
    cout << Dist(A, B);
    return 0;
}

Comments

[Модератор]

Evgeny дорогой пользователь, настоятельно рекомендуем еще раз обратить самое пристальное внимание на п. 3.1 регламента работы сервиса (и, в особенности, на его последний абзац).
В противном случае, ваши вопросы будут удаляться по причине тег-спама, а систематические нарушения приведут к блокировке учетной записи.

Answer 1

[laphroaig]

При передаче объекта по значению происходит его копирование, но динамически выделенная память не копируется, а только значение указателя _coords. В результате у вас два объекта которые ссылаются на один и тот же массив. По завершению работы Dist копия объекта уничтожается с вызовом деструктора и освобождением памяти. А по завершению работы программы вызываются деструкторы объектов A и B в которых происходит попытка освободить память для _coords второй раз, а это неопределенное поведения (undefined behaviour) другими словами может произойти вообще что угодно.

Вам нужно самому реализовать конструктор копирования или запретить его. Когда вы просто удаляете деструктор происходит утечка памяти, когда передаете по ссылке объекты не копируются поэтому всё норм. Если объект динамически выделяет память, например с помощью new или delete, то реализация конструктора копирования обязательна. Можно также его запретить перенеся конструктор копирования в приватную секцию или для c++11 объявить Point(const Point&) = delete; В этом случае передача такого объекта по значению приведет к ошибке компиляции. Это важная тема изучите ее.

Все что выделено с помощью new [] удаляется только с помощью delete[]

Comments

[Clean Coder]

Прямо все по полочкам разложили. Спасибо!

Answer 2

[Vitaly]

У вас же C++, т.е. можете использовать вектор для хранения элементов без необходимости явного выделения памяти:

#include <vector>

// double* _coords;
std::vector<double> _coords;

// _coords = new double[N];
_coords.resize(N);

Ну и деструктор можете не писать.

Answer 3

[res2001]

delete[] _coords;
В таком виде в Dist создаются копии объектов с помощью конструктора копирования по умолчанию, т.е. просто побайтовое копирование. После завершения Dist память под coords в копиях освобождается. Когда завершается main() та же самая память будет освобождена еще раз, что приведет к ошибке.
Поэтому в Dist передавайте ссылки!

Answer 4

[Dmitry]

Используйте delete [].

Comments

[Clean Coder]

в чем разница?

[Kirgus]

Clean Coder, delete освобождает только одну ячейку по указателю, а delete[] освобождает весь выделенный массив.