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

Question

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

По какой-то причине, когда я удалю 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; 
    } 
} 

Answer 1

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

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

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

Это не огромная сделка, но несогласованные круглые скобки являются врагом.

Answer 2

В вашей push функции, в else блоке вы

currentArray = newArray;

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

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

Кроме того, в вашем нажимной функции вы ::

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

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

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

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

внутри, если блок, я полагаю, вы копируете данные 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, вот почему: What are the advantages of using nullptr?

Answer 3

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

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

Толчок имеет несколько проблем:

1. Проходите value по значению, которое может быть дорогим. Вы должны пройти по ссылке.
2. Вы используете size в copy() после его увеличения, что означает, что вы копируете необработанную память в последний слот newArray. Это может быть доброкачественным (T = int + немного удачи) или катастрофическим (T = std :: string).
3. newArray необходимо только внутри push(). Он должен быть локальной переменной, а не переменной-членом.
4. Вы увеличиваетесь на 1 каждый раз, что приводит к O (n) время для заполнения. Вы должны расти геометрически, что требует дополнительной переменной элемента емкости.
5. Вы дважды вызываете 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.

Answer 4

@ пользователь007 это мой 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; 
}