patterncppModerate
Linked List implementation in C++
Viewed 0 times
listimplementationlinked
Problem
I'm learning to implement linked list. I've implemented sorting with merge sort, reversing etc...
LinkedList.cpp
```
#include
template
struct Node
{
T data;
Node * next;
};
template
class LinkedList
{
public:
LinkedList() : head(NULL), size(0) {};
~LinkedList() { destroyList(); };
bool addNode(T data);
bool deleteNode(T data);
Node * searchNode(T data);
void printList();
void reverseList();
void sortList();
private:
Node * head;
int size;
void destroyList();
Node mergeSort(Node head, int total);
Node Merge(Node left, int lcount, Node* right, int rcount);
void print(Node * tmp);
};
template
bool LinkedList::addNode(T data)
{
try
{
Node * tmp = new Node();
tmp->data = data;
tmp->next = head;
head = tmp;
++size;
return true;
}
catch(std::exception & ex)
{
return false;
}
}
template
bool LinkedList::deleteNode(T data)
{
Node curr = head, prev = NULL;
while (curr)
{
if (curr->data == data) break;
prev = curr;
curr = curr->next;
}
if (curr)
{
if (prev)
{
prev->next = curr->next;
}
else
{
head = curr->next;
}
delete(curr);
--size;
return true;
}
else
{
return false;
}
}
template
Node * LinkedList::searchNode(T data)
{
Node * tmp = head;
while (tmp)
{
if (tmp->data == data)
{
return tmp;
}
tmp = tmp->next;
}
return NULL;
}
template
void LinkedList::print(Node * tmp)
{
bool printNewLine = (tmp) ? true : false;
while (tmp)
{
std::cout data next;
}
if (printNewLine)
{
std::cout
void LinkedList::printList()
{
Node * tmp = head;
bool printNewLine = (tmp) ? true : false;
while (tmp)
{
std::cout data next;
}
if (printNewLine)
{
std::cout
void LinkedList::destroyList()
{
Node * tmp = NULL;
while (head)
{
tmp = head;
head = head
LinkedList.cpp
```
#include
template
struct Node
{
T data;
Node * next;
};
template
class LinkedList
{
public:
LinkedList() : head(NULL), size(0) {};
~LinkedList() { destroyList(); };
bool addNode(T data);
bool deleteNode(T data);
Node * searchNode(T data);
void printList();
void reverseList();
void sortList();
private:
Node * head;
int size;
void destroyList();
Node mergeSort(Node head, int total);
Node Merge(Node left, int lcount, Node* right, int rcount);
void print(Node * tmp);
};
template
bool LinkedList::addNode(T data)
{
try
{
Node * tmp = new Node();
tmp->data = data;
tmp->next = head;
head = tmp;
++size;
return true;
}
catch(std::exception & ex)
{
return false;
}
}
template
bool LinkedList::deleteNode(T data)
{
Node curr = head, prev = NULL;
while (curr)
{
if (curr->data == data) break;
prev = curr;
curr = curr->next;
}
if (curr)
{
if (prev)
{
prev->next = curr->next;
}
else
{
head = curr->next;
}
delete(curr);
--size;
return true;
}
else
{
return false;
}
}
template
Node * LinkedList::searchNode(T data)
{
Node * tmp = head;
while (tmp)
{
if (tmp->data == data)
{
return tmp;
}
tmp = tmp->next;
}
return NULL;
}
template
void LinkedList::print(Node * tmp)
{
bool printNewLine = (tmp) ? true : false;
while (tmp)
{
std::cout data next;
}
if (printNewLine)
{
std::cout
void LinkedList::printList()
{
Node * tmp = head;
bool printNewLine = (tmp) ? true : false;
while (tmp)
{
std::cout data next;
}
if (printNewLine)
{
std::cout
void LinkedList::destroyList()
{
Node * tmp = NULL;
while (head)
{
tmp = head;
head = head
Solution
#include
template
struct Node
{
T data;
Node * next;
};
template
class LinkedList
{
public:
LinkedList() : head(NULL), size(0) {};
~LinkedList() { destroyList(); };Why did you make destroyList a seperate method? Why not put it in the destructor?
bool addNode(T data);
bool deleteNode(T data);
Node * searchNode(T data);
void printList();
void reverseList();
void sortList();
private:
Node * head;
int size;I recommend some newlines between the data methods and the utility functions.
void destroyList();
Node* mergeSort(Node * head, int total);
Node* Merge(Node* left, int lcount, Node* right, int rcount);I recommend picking a consistent capitalization scheme. This should be merge not Merge.
void print(Node * tmp);
};
template
bool LinkedList::addNode(T data)
{
try
{
Node * tmp = new Node();
tmp->data = data;
tmp->next = head;
head = tmp;
++size;
return true;
}
catch(std::exception & ex)
{
return false;
}Don't ever do this. You shouldn't generally catch all exceptions. You should catch only the exception you are interested in. Converting the exception into a return value loses the advantage of an exception. Why did you add this exception logic here?
}
template
bool LinkedList::deleteNode(T data)
{
Node *curr = head, *prev = NULL;I recommend typing whole words not abbreviations.
while (curr)
{
if (curr->data == data) break;Use
while(curr && curr->data != data)prev = curr;
curr = curr->next;
}
if (curr)
{You are making use of inconsistent indentation. Pick a scheme and stick to it.
if (prev)
{
prev->next = curr->next;
}
else
{
head = curr->next;
}
delete(curr);Parenthesis not needed
--size;
return true;
}
else
{
return false;
}
}
template
Node * LinkedList::searchNode(T data)You are really finding a node rather then searching it.
{
Node * tmp = head;tmp should be a banned variable name. all variables are temporary, so pick a better name,
while (tmp)
{I suggest
for(Node * tmp = head; tmp; tmp = tmp->next)
I think it'll make the code clearer.
if (tmp->data == data)
{
return tmp;
}
tmp = tmp->next;
}
return NULL;
}
template
void LinkedList::print(Node * tmp)Printing is a bit of a odd feature for a linked list class. Typically, you'd except external code to handle that.
{
bool printNewLine = (tmp) ? true : false;Use 'bool printNewLine = bool(tmp)';
while (tmp)
{
std::cout data next;
}
if (printNewLine)Rather then doing this. I suggest keeping a copy of the original parameter and testing it here.
{
std::cout next;
}
count = total/2;Rather then doing this calculation twice, I suggest saving the original version
first = mergeSort(first, count);
curr = mergeSort(curr, total-count);
return Merge(first, count, curr, total-count);
}
template
Node* LinkedList::Merge(Node* left, int lcount, Node* right, int rcount)
{
Node * h = new Node();
h->next = NULL;
Node * tmp = h;Creating a node during merging seems odd...
while (lcount > 0 && rcount > 0)
{
if (left->data data)
{
tmp->next = left;
tmp = tmp->next;use
tmp = leftleft = left->next;
--lcount;
}
else if (right->data data)
{
tmp->next = right;
tmp = tmp->next;
right = right->next;
--rcount;
}
else
{
tmp->next = left;
tmp = tmp->next;
left = left->next;
--lcount;
tmp->next = right;
tmp = tmp->next;
right = right->next;
--rcount;There is no need for this. Just let one of the above cases handle equal as well.
}
}
while (lcount > 0)
{
tmp->next = left;
tmp = tmp->next;
left = left->next;
--lcount;This pattern is getting common. Think about refactoring this function or writing a function.
}
while (rcount > 0)
{
tmp->next = right;
tmp = tmp->next;
right = right->next;
--rcount;
}
tmp = h;
h = h->next;
delete(tmp);
return h;
}EDIT: My version of merge
```
template
Node LinkedList::Merge(Node left, int count_left, Node* right, int count_right)
{
Node * head = NULL;
Node ** current = &head;
while (count_left > 0 || count_right > 0)
{
if( count_right == 0 || (count_left > 0 && left->data data))
{
*current = left;
left = left->next;
--count_left;
}
else
{
*current = right;
right = right->next;
--count_right;
}
current = &(*curren
Code Snippets
#include <iostream>
template <class T>
struct Node
{
T data;
Node * next;
};
template <class T>
class LinkedList
{
public:
LinkedList() : head(NULL), size(0) {};
~LinkedList() { destroyList(); };bool addNode(T data);
bool deleteNode(T data);
Node<T> * searchNode(T data);
void printList();
void reverseList();
void sortList();
private:
Node<T> * head;
int size;void destroyList();
Node<T>* mergeSort(Node<T> * head, int total);
Node<T>* Merge(Node<T>* left, int lcount, Node<T>* right, int rcount);void print(Node<T> * tmp);
};
template <class T>
bool LinkedList<T>::addNode(T data)
{
try
{
Node<T> * tmp = new Node<T>();
tmp->data = data;
tmp->next = head;
head = tmp;
++size;
return true;
}
catch(std::exception & ex)
{
return false;
}template <class T>
bool LinkedList<T>::deleteNode(T data)
{
Node<T> *curr = head, *prev = NULL;Context
StackExchange Code Review Q#4569, answer score: 10
Revisions (0)
No revisions yet.