C ++ Массив указателей: удалить или удалить []? - PullRequest
37 голосов
/ 12 мая 2010

Cosider следующий код:

class Foo
{
    Monster* monsters[6];

    Foo()
    {
        for (int i = 0; i < 6; i++)
        {
            monsters[i] = new Monster();
        }
    }

    virtual ~Foo();
}

Какой правильный деструктор?

это:

Foo::~Foo()
{
    delete [] monsters;
}

или это:

Foo::~Foo()
{
    for (int i = 0; i < 6; i++)
    {
        delete monsters[i];
    }
}

В настоящее время у меня есть самый верхний конструктор, и все работает хорошо, но, конечно, я не вижу, протекает ли это ...

Лично я думаю, что вторая версия гораздо более логична, учитывая то, что я делаю. Во всяком случае, каков «правильный» способ сделать это?

Ответы [ 8 ]

43 голосов
/ 12 мая 2010

delete[] monsters;

Неверно, потому что monsters не является указателем на динамически размещенный массив, он является массивом указателей. Как член класса он будет уничтожен автоматически при уничтожении экземпляра класса.

Ваша другая реализация является правильной, так как указатели в массиве указывают на динамически размещенные Monster объекты.

Обратите внимание, что с вашей текущей стратегией выделения памяти вы, вероятно, захотите объявить свой собственный конструктор копирования и оператор копирования-назначения, чтобы непреднамеренное копирование не вызывало двойного удаления. (Если вы хотите предотвратить копирование, вы можете объявить их как частные и не реализовывать их.)

37 голосов
/ 12 мая 2010

Для new вы должны использовать delete. Для new[] используйте delete[]. Ваш второй вариант верен.

11 голосов
/ 20 ноября 2012

Чтобы упростить ответ, давайте посмотрим на следующий код:

#include "stdafx.h"
#include <iostream>
using namespace std;

class A
{
private:
    int m_id;
    static int count;
public:
    A() {count++; m_id = count;}
    A(int id) { m_id = id; }
    ~A() {cout<< "Destructor A "   <<m_id<<endl; }
};

int A::count = 0;

void f1()
{   
    A* arr = new A[10];
    //delete operate only one constructor, and crash!
    delete arr;
    //delete[] arr;
}

int main()
{
    f1();
    system("PAUSE");
    return 0;
}

Вывод: Деструктор А 1 а затем происходит сбой (выражение: _BLOCK_TYPE_IS_VALID (phead- nBlockUse)).

Нам нужно использовать: delete [] arr; потому что это удаляет весь массив, а не только одну ячейку!

попробуйте использовать delete [] arr; выход: Деструктор А 10 Деструктор А 9 Деструктор А 8 Деструктор А 7 Деструктор А 6 Деструктор А 5 Деструктор А 4 Деструктор А 3 Деструктор А 2 Деструктор А 1

Тот же принцип для массива указателей:

void f2()
{
    A** arr = new A*[10];
    for(int i = 0; i < 10; i++)
    {
        arr[i] = new A(i);
    }
    for(int i = 0; i < 10; i++)
    {
        delete arr[i];//delete the A object allocations.
    }

    delete[] arr;//delete the array of pointers
}

если мы будем использовать delete arr вместо delete [] arr. он не удалит целые указатели в массиве => утечка памяти из объектов указателя!

11 голосов
/ 12 мая 2010

Второй вариант является верным в данных обстоятельствах (ну, во всяком случае, наименее неправильным).

Редактировать: "наименее неправильно", так как в исходном коде нет веских причин использовать new или delete, поэтому вам, вероятно, следует просто использовать:

std::vector<Monster> monsters;

В результате будет проще код и четкое разделение обязанностей.

6 голосов
/ 12 мая 2010

delete[] monsters определенно не так. Мой отладчик кучи показывает следующий вывод:

allocated non-array memory at 0x3e38f0 (20 bytes)
allocated non-array memory at 0x3e3920 (20 bytes)
allocated non-array memory at 0x3e3950 (20 bytes)
allocated non-array memory at 0x3e3980 (20 bytes)
allocated non-array memory at 0x3e39b0 (20 bytes)
allocated non-array memory at 0x3e39e0 (20 bytes)
releasing     array memory at 0x22ff38

Как вы можете видеть, вы пытаетесь выпустить неправильную форму удаления (не массив или массив), и указатель 0x22ff38 никогда не возвращался при вызове new. Вторая версия показывает правильный вывод:

[allocations omitted for brevity]
releasing non-array memory at 0x3e38f0
releasing non-array memory at 0x3e3920
releasing non-array memory at 0x3e3950
releasing non-array memory at 0x3e3980
releasing non-array memory at 0x3e39b0
releasing non-array memory at 0x3e39e0

В любом случае, я предпочитаю проект, в котором ручная реализация деструктора не обязательна для начала.

#include <array>
#include <memory>

class Foo
{
    std::array<std::shared_ptr<Monster>, 6> monsters;

    Foo()
    {
        for (int i = 0; i < 6; ++i)
        {
            monsters[i].reset(new Monster());
        }
    }

    virtual ~Foo()
    {
        // nothing to do manually
    }
};
3 голосов
/ 12 мая 2010

Ваш второй пример верен; вам не нужно удалять сам массив monsters, только отдельные созданные вами объекты.

1 голос
/ 12 мая 2010

Было бы разумно, если бы ваш код был таким:

#include <iostream>

using namespace std;

class Monster
{
public:
        Monster() { cout << "Monster!" << endl; }
        virtual ~Monster() { cout << "Monster Died" << endl; }
};

int main(int argc, const char* argv[])
{
        Monster *mon = new Monster[6];

        delete [] mon;

        return 0;
}
0 голосов
/ 12 мая 2010

Вы удаляете каждый указатель отдельно, а затем удаляете весь массив. Убедитесь, что вы определили правильный деструктор для классов, хранящихся в массиве, иначе вы не можете быть уверены, что объекты очищены должным образом. Убедитесь, что все ваши деструкторы являются виртуальными, чтобы они работали правильно при использовании с наследованием.

...