Reputation: 31
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
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
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