GloriousDemonFist
GloriousDemonFist

Reputation: 31

Will there be a memory leak in this Linked list

Hi Currently i have been honing my skill in the data structure so that i can become a good Game Developer, i am Learning Linked list and made a Linked list Program with Insertion ,Deletion and Recursive insertion and Reversing a Linked list Can you Guys tell me am i here Clearing the assigned memory Created with the new Operator Correctly, I am getting the Desired Output but i am worried about memory leak. ... please be gentle aim Still learning.

class Node
{
    int data;
    Node *Next;

public:
    int GetData()
    {
        return data;
    }
    void SetData(int Data)
    {
        data = Data;
    }

    Node *GetNext()
    {
        return Next;
    }

    void SetNext(Node *next)
    {
        Next = next;
    }
};

void Insert(Node **Head, int x)
{
    Node *temp = new Node();

    temp->SetData(x);
    temp->SetNext(*Head);

    *Head = temp;
}

void InsertAt(Node **Head, int x, int n)
{
    if (n == 0)
    {
        std::cout << "The Given data at 'n' cannot be assigned to null\n";
    }

    Node *temp = new Node();
    temp->SetData(x);
    if (n == 1)
    {
        temp->SetNext(nullptr);
        *Head = temp;
        return;
    }

    Node *temp2 = *Head;
    if (Head == nullptr)
    {
        std::cout << "The Given data cannot be assigned to null\n";
    }

    for (int i = 0; i < n - 2; i++)
    {
        temp2 = temp2->GetNext();
    }

    temp->SetNext(temp2->GetNext());
    temp2->SetNext(temp);
}

void AppendList(Node **Head, int Data)
{
    Node *temp = new Node();

    Node *LastNode = *Head;

    temp->SetData(Data);
    temp->SetNext(nullptr);

    if (*Head == nullptr)
    {
        *Head = temp;
        return;
    }
    // else Traverse till last node.

    while (LastNode->GetNext() != nullptr)
    {
        LastNode = LastNode->GetNext();
    }
    LastNode->SetNext(temp);
}

void DeleteNode(Node **Head, int n)
{
    Node *temp = *Head;

    if (n == 1)
    {
        *Head = temp->GetNext();
        std::cout << "\nAfter Deletion of Head Node\n";
        return;
    }

    for (int i = 0; i < n - 2; i++)
    {
        temp = temp->GetNext();
    }
    Node *temp2 = temp->GetNext();
    temp->SetNext(temp2->GetNext());
    std::cout << "After Deletion of Node\n";
}

Node *ReverseList(Node *Head)
{
    Node *Current, *Prev, *next;

    Current = Head;
    Prev = nullptr;
    while (Current != nullptr)
    {
        next = Current->GetNext();
        Current->SetNext(Prev);
        Prev = Current;
        Current = next;
    }
    Head = Prev;
    return Head;
}

int LinkedList_Count(Node *Head)
{
    int count = 0;

    Node *Current = Head;
    while (Current != nullptr)
    {
        count++;
        Current = Current->GetNext();
    }
    std::cout << "Number of Elements: " << count;
    return count;
}

void PrintList(Node *Head)
{
    std::cout << "Data list : ";

    while (Head != nullptr)
    {
        std::cout << " " << Head->GetData();
        Head = Head->GetNext();
    }
    std::cout << "\n";
}

//Print Listed using Recursion
void Recursion_Print(Node *Head)
{
    if (Head == nullptr)
    {
        return;
    }

    std::cout << ' ' << Head->GetData(); //comment to Do Reverse the Linked list
    Recursion_Print(Head->GetNext());
    //std::cout << ' ' << Head->GetData();//unComment to Reverse the linked List recursively
}

Node *RecursiveRevList(Node *Head)
{
    Node *temp;
    if (Head->GetNext() == nullptr)
    {
        temp = Head;
        return temp;
    }
    temp = RecursiveRevList(Head->GetNext());
    Head->GetNext()->SetNext(Head);
    Head->SetNext(nullptr);
    return temp;
}
void RunList()
{
    Node *Head = NULL;

    //AppendList(&Head, 16);
    Insert(&Head, 6);
    Insert(&Head, 7);
    Insert(&Head, 8);
    InsertAt(&Head, 18, 2);
    std::cout << "Data list : \n";
    Recursion_Print(Head);
    std::cout << '\n';
    LinkedList_Count(Head);
    DeleteNode(&Head, 1);
    //Head = ReverseList(Head);
    Head = RecursiveRevList(Head);
    PrintList(Head);
    LinkedList_Count(Head);
    delete Head;
}

Upvotes: 1

Views: 179

Answers (2)

&#208;аn
&#208;аn

Reputation: 10875

You're writing C-style code; in C++, you should avoid explicit calls to new. Your example is far too long to rewrite, but here is a very small start:

#include <memory>
class Node
{
    int data;
    std::shared_ptr<Node> Next;

// ...
void Insert(std::shared_ptr<Node>& Head, int x)
{
    auto temp = std::make_shared<Node>();
    // ...
}

(Note that std::unique_ptr is probably a better choice than std::shared_ptr, but that incurs complications with copying Node of which you're "blissfully" unaware right now.)

And, in practice, you should really use std::list (in your case, std::list<int>) rather than writing your own. Once you're proficient using std::list (and friends like std::vector), you'll be better able to "roll your own."

Upvotes: 4

Aman Kumar
Aman Kumar

Reputation: 324

As pointed out in the comments by many learned people, you have a memory leak in your program. When you are deleting the nodes, you're not actually freeing the allocated memory locations. The correct way? Use delete to deallocate the memory from the program.

I would recommend you to learn it as a rule of thumb, when programming in C or C++, if you're allocating dynamic memory somewhere in your program, then in all certainty you'd have some deletion method where you should use free() or delete to deallocate the memory from the heap.

Here are some resources which might help you.

Upvotes: 3

Related Questions