C ++ Как удалить узел без потери памяти?

Question

Я просто не могу заставить этот единственный связанный список работать без потери памяти. Я погуглил и насколько я вижу, я делаю то, что предлагает большинство. Я также попытался «бесплатно (deleteNode)» вместо «удалить deleteNode». Кто-нибудь думает, что знает ответ?

Заранее спасибо.

Я пытаюсь вставить и удалить с помощью этого кода.

List l{};
l.insert(5);
l.remove(5);

Держу пари, ошибка в моей функции remove:

void List::remove(int input){

    if(top -> getValue() == input){
        Node * deleteNode = new Node;
        deleteNode = top;
        top = top -> getNext();
        delete deleteNode;
        amount--;
        return;
    }

    Node * tmpNode;
    tmpNode = new Node(top);

    while(tmpNode -> getValue() != 0){
        if(tmpNode -> getNext() -> getValue() == input){
            Node * deleteNode;
            deleteNode = new Node(tmpNode -> getNext());
            tmpNode -> setNext(deleteNode -> getNext());
            deleteNode -> setNext(nullptr);
            delete deleteNode;
            amount--;
            return;
        }
        tmpNode = tmpNode -> getNext();
    }
}

Мой cc файл:

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

List::List() : amount(0), top(nullptr) {
}

List::List(Node* input) : List(){
    top = input;
}

List::~List(){//destructor
    while( top != nullptr){
        remove( top -> getValue());
    }
}

List::Node::~Node(){//destructor
    next = nullptr;
    //this =NULL;
}

    List::List(List const& other) : List(){
        *this = other;
    }

    List::List(List && other) : List(){ // move constructor
       Node* tmpNode = other.top;
       other.top = top;
       top = tmpNode;
       int tmpAmount = size();
        setSize(other.size());
        other.setSize(tmpAmount);
    }

    List & List::operator=(List && other){ // move assignment
        Node* tmpNode = other.top;
        other.top = top;
        top = tmpNode;
        int tmpAmount = size();
        other.size();
        setSize(other.size());
        other.setSize(tmpAmount);
        return *this;
    }

    List & List::operator=(List const& other){// copy assignment
        Node * tmpNode; 
        tmpNode = other.top;

        while(tmpNode != nullptr){
            insert(tmpNode -> getValue());
            tmpNode = tmpNode -> getNext();
        }

        return *this;
    }

void List::setSize(int input){
    amount = input;
}

void List::insert(int input){
    Node* newNode;
    newNode = new Node(input);
    if(!top){
        top = newNode;
        amount++;
        return;
    }

    if(input > top -> getValue()){
        newNode -> setNext(top);
        top = newNode;
        amount++;
        return;
    }

    top -> putIterator(newNode);
    amount++;
    return;
}

string List::print(){
    if(top == nullptr){
        return "";
    }
    string output = to_string(top -> getValue());
    if(top -> getNext() == nullptr){
        return output;
    }
    return top -> getNext() -> print(output);
}

void List::remove(int input){

    if(top -> getValue() == input){
        Node * deleteNode = new Node;
        deleteNode = top;
        top = top -> getNext();
        delete deleteNode;
        amount--;
        return;
    }

    Node * tmpNode;
    tmpNode = new Node(top);

    while(tmpNode -> getValue() != 0){
        if(tmpNode -> getNext() -> getValue() == input){
            Node * deleteNode;
            deleteNode = new Node(tmpNode -> getNext());
            tmpNode -> setNext(deleteNode -> getNext());
            deleteNode -> setNext(nullptr);
            delete deleteNode;
            amount--;
            return;
        }
        tmpNode = tmpNode -> getNext();
    }
}

List::Node List::find(int input){
    return iterate(input).getNext();
}

List::Node List::iterate(int input){
    return top -> iterate(input);
}

bool List::isEmpty(){
    if(size()==0){
        return true;
    }
    return false;
}

int List::size(){
    return amount;
}

List::Node::Node(int input, Node &nextInput){
    value = input;
    next = &nextInput;
}

List::Node::Node(int input){
    value = input;
    next = nullptr;
}

List::Node::Node(const Node* input){
    *this = *input;
}

List::Node* List::Node::getNext(){
    return next;
}

void List::Node::setNext(Node* input){
    next = input;
}

int List::Node::getValue(){
    return value;
}

/*
void List::Node::deleteNode(){
    delete *this;
}*/

void List::Node::putIterator(Node* newNode){
    if (next == nullptr){
        next = newNode;
        next -> setNext(nullptr);
        return;
    }

    if(getValue() == newNode -> getValue()){
        newNode -> setNext(getNext());
        setNext(newNode);
        return; 
    }

    if(next -> value < newNode -> value && value > newNode -> value){
        newNode -> setNext(getNext());
        setNext(newNode);
        return;
    }

    next -> putIterator(newNode);
    return;
}

string List::Node::print(string input){
    input = input + ", " + to_string(value);
    if(next == nullptr){
        return input;
    }
    return next -> print(input);
}

List::Node List::Node::iterate(int input){
    if (next -> value==input){
        return *this;
    }
    if (next -> value==0){
        return nullptr;
    }

    return next ->iterate(input);
}

bool List::Node::operator!() const{
    if(value == 0){
        return true;
    }
    return false;
}

Мой заголовочный файл:

#ifndef _LIST_H_
#define _LIST_H_
#include <string>

class List
{
  public:
    List();

    ~List(); //destructor
    List(List const &other);
    List(List &&other);                 // move constructor
    List &operator=(List &&other);      // move assignment
    List &operator=(List const &other); // copy assignment

  class Node
    {
      public:
        Node() = default;
        ~Node();
        Node(int input, Node &nextInput);
        Node(int input);
        Node(const Node *input);
        Node *getNext();
        void setNext(Node *input);
        int getValue();
        Node iterate(int input);
        void putIterator(Node *newNode);
        void deleteNode();
        bool operator!() const;
        std::string print(std::string input);

      private:
        int value;
        Node *next;
    };

    List(Node* input);
    void insert(int input);
    void remove(int input);
    Node iterate(int input);
    int size();
    bool isEmpty();
    Node find(int input);
    std::string print();
    void setSize(int input);

   private:

    Node *top;
    int amount;
};

#endif

Answer 1

Почему вы вообще создаете новый узел в remove()?

Далее, добавление геттеров, сеттеров и всего прочего дерьма в Node (который является подробностью реализации List) просто усложняет List. Удалите все это (кроме ctor, возможно).

Рассмотрите возможность использования двойного косвенного обращения или других способов устранения особых случаев и получающегося в результате подверженного ошибкам дублирования:

void List::remove(int x) {
    auto p = ⊤
    while (*p && p[0]->value != x)
        p = &p[0]->next;
    if (*p)
        delete std::exchange(*p, p[0]->next);
}

Альтернатива:

void List::remove(int x) {
    auto curr = top;
    curr = nullptr;
    auto next = top;
    while (next && next->value != x) {
        curr = next;
        next = next->next;
    }
    if (!next)
        return;
    (curr ? curr->next : top) = next->next;
    delete next;
}

Answer 2

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

Node * deleteNode = new Node;

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

Новая функция - выделять кучу памяти, поэтому вызывайте ее только тогда, когда вы решите выделить больше памяти.

Очень простое решение - не вызывать new при объявлении объекта. Попробуйте что-то вроде:

void List::remove(int input){

    if(top -> getValue() == input){
        Node * deleteNode;
        deleteNode = top;
        top = top -> getNext();
        delete deleteNode;
        amount--;
        return;
    }

    Node * tmpNode;
    tmpNode = top;

    while(tmpNode -> getValue() != 0){
        if(tmpNode -> getNext() -> getValue() == input){
            Node * deleteNode;
            deleteNode = tmpNode -> getNext());
            tmpNode -> setNext(deleteNode -> getNext());
            deleteNode -> setNext(nullptr); // you could probably delete this line
            delete deleteNode;
            amount--;
            return;
        }
        tmpNode = tmpNode -> getNext();
    }

}