Удаление массивов или освобождение памяти, ошибка C ++

Question

Удаление массивов или освобождение памяти, ошибка C ++

Ниже приведен фрагмент моего кода для структуры данных стека, которую я пытаюсь реализовать.

По какой-то причине, когда я удаляю currentArray, newArray также удаляется, потому что код ниже дает мне ошибку во время выполнения, где содержимое newArray а также currentArray значения мусора.

Я не уверен, почему это происходит.

Буду очень признателен за понимание того, почему я получаю эту ошибку, и если мой push() нижеприведенная реализация является принципиальной с точки зрения

// Destructor
~Stack()
{
if ((size > 0) && (currentArray) != NULL && (newArray != NULL))
{
delete[] currentArray;
delete[] newArray;
}
}

inline void push(T value)
{
if (isEmpty())
{
// Stack size is increased by 1
size++;

currentArray = new T[size];
currentArray[size - 1] = value;
}
else
{
// Stack size is increased by 1
size++;

// Create the new array
newArray = new T[size];

// Copy the contents of the old array into the new array
copy(currentArray, currentArray + size, newArray);

// Push the new value on to the new array
newArray[size - 1] = value;

// Copy the new array into the current
currentArray = new T[size];
currentArray = newArray;
}
}

0

c++data-structures dealloc memory-management stack

Решение

Другие решения

В вашем push функция, в else заблокировать вы делаете

currentArray = newArray;

Так что оба ваши currentArray а также newArray указывают на то же самое! (что вы делаете сами).

Итак, когда вы делаете delete [] currentArray вам не нужно удалять newArray так как эта память уже была удалена. И именно поэтому ваша программа выдает ошибку, что вы пытаетесь удалить память, которая уже была удалена !!

Более того, в вашей функции push вы делаете:

currentArray = new T[size];
currentArray = newArray;

Я верю в твои class, currentArray это pointer а также new также возвращает указатель на вновь выделенную память. Итак, если вы хотите, чтобы ваш currentArray указать на newArray, тебе просто нужно сделать currentArray = newArray без выделения памяти для currentArray,

Кроме того, вы также делаете ::

 newArray = new T[size];
copy(currentArray, currentArray + size, newArray);

внутри вашего блока if, я полагаю, вы копируете данные currentArray к newArray Итак, после того, как вы скопировали данные из currentArray в newArray вам нужно delete ранее выделенная память для currentArrayиначе это утечка памяти (Ошибка в программе освободить удаленную память, что приводит к снижению производительности или сбою). Это всегда хорошо, чтобы delete память, которая бесполезна.

Итак, я бы рекомендовал вам добавить delete [] currentArray заявление после этого.

Итак, ваш окончательный код должен выглядеть примерно так:

// Destructor
~Stack()
{
/*
*if ((size > 0) && (currentArray) != NULL) -->
*deleting a NULL pointer is completely defined!
*So, in your constructor if you are initializing
*currentArray to NULL you need not have any if condition.
*/
delete[] currentArray;
}

inline void push(T value)
{
if (isEmpty())
{
size++;

currentArray = new T[size];
currentArray[size - 1] = value;
}
else
{
size++;

newArray = new T[size];

copy(currentArray, currentArray + size, newArray);

delete [] currentArray; //EDIT

newArray[size - 1] = value;

//currentArray = new T[size]; --> Deleted Line
currentArray = newArray;
}
}

НОТА :: На всякий случай, если ваш компилятор соответствует C ++ 11, вы должны рассмотреть возможность использования nullptr над NULLвот почему: Каковы преимущества использования nullptr?

2

Может быть, я что-то упустил, но если if утверждение подтверждается, оно удаляет оба так или иначе.

Кроме того, с основной точки зрения, почему бы вам не иметь:

if ((size > 0) && (currentArray != NULL) && (newArray != NULL))

Это не большое дело, но несоответствующие скобки — враг.

0

@ user007 это моя функция pop ().

// Remove an element from the stack
inline const T& pop()
{
if (size > 0)
{
T value;

size--;

value = data[size];

T* dest = new T[size];

// Copy the contents of the old array into the new array
copy(data, data + (size), dest);

delete[] data;

data = dest;

return value;
}

return NULL;
}

0

Источник

Accepted Answer

Во-первых, вам не нужны проверки в деструкторе.

~Stack() {
delete [] currentArray;
delete [] newArray;
}

У толчка есть несколько проблем:

Вы передаете value по стоимости, которая может быть дорогой. Вы должны пройти по ссылке.
Ты используешь size в copy() после увеличения, это означает, что вы копируете необработанную память в последний слот newArray, Это может быть доброкачественным (T = int + немного удачи) или катастрофическим (T = std :: string).
newArray нужен только внутри push(), Это должна быть локальная переменная, а не переменная-член.
Вы растете на 1 каждый раз, что приводит к O (N²) время заселять. Вы должны расти геометрически, что требует дополнительной переменной члена емкости.
Вы вызываете new T[size] дважды. Один из них протекает.

Вот переработанная версия:

class Stack {
public:
…
inline void push(const T& value) {
if (size_ == capacity_) {
capacity_ = capacity_ ? capacity_ * 2 : 1;
T* dest = new T[capacity_];
copy(data_, data_ + size_, dest);
delete [] data_;
data_ = dest;
}
data_[size_++] = value;
}
…
private:
T* data_ = nullptr;
size_t size_ = 0, capacity_ = 0;
};

Это ни в коем случае не твердый, промышленный код. Это не относится к безопасности исключений, предотвращению копирования (помимо прочего, требуется дополнительная перегрузка: push(T&& value)) и ряд других тонкостей. На самом деле, я даже не проверял, что он компилируется! Это может сделать для игрушечного проекта, но в реальном мире вы должны просто использовать std :: stack.

2