#include <iostream>
using namespace std;

template <class T>
struct Node
{
T data;
Node * next;
Node(T data) : data(data), next(NULL) {}
};

template <class T>
{
public:
void deleteNode(T data);
template <class U>
friend std::ostream & operator<<(std::ostream & os, const CircularLinkedList<U> & cll);
private:
};

template <class T>
{
{
{
Node<T> * t = tmp;
tmp = tmp->next;
delete(t);
}
delete tmp;
}
}

template <class T>
{
Node<T> * t = new Node<T>(data);

{
t->next = t;
return;
}

{
tmp = tmp->next;
}

tmp->next = t;
}

template <class T>
{
Node<T> * prev = NULL;
{
if (tmp->data == data) break;
prev = tmp;
tmp = tmp->next;
}

{
{
tmp = tmp->next;
}
}
else
{
prev->next = tmp->next;
delete tmp;
}
}

template <class U>
std::ostream & operator<<(std::ostream & os, const CircularLinkedList<U> & cll)
{
{
{
os << tmp->data << " ";
tmp = tmp->next;
}
os << tmp->data;
}
return os;
}

int main()
{

cout << cll << endl;

cll.deleteNode(3);
cll.deleteNode(1);
cll.deleteNode(5);

cout << cll << endl;

return 0;
}

-
Most of the cases I wrote for your other question are also applicable here: codereview.stackexchange.com/questions/4629/… –  loki2302 Sep 7 '11 at 6:33
Use const reference instead of value –  fasked Sep 6 '12 at 21:47

)

My first comment is you should start looking at smart pointers.
They will definitely simplify your code.

The second comment: look at the STL containers.
Containers in C++ follow a specific pattern. The idea is that we want to use algorithms on containers interchangeably and thus containers follow these conventions to make them easy to use with the standard algorithms.

Third comment: you fail to follow the rule of three.
Basically. If you define one of 'Destructor'/'Copy-Constructor'/'Assignment-Operator' (rather than using the compiler generated defaults) then you probably need to define all three. In this case your code is going to crash if you make a copy of the list. The problem arises from having an owned pointer in your code.

Fourth comment: Prefer to be pass parameters by const reference. For simple objects like integers it will not make any difference. But you have templatised the code and T can be any type. Thus passing by value is going to cause a copy (which may be expensive).

Reviewing the above code:
The complexity of adding a node is O(n). By maintaining a head and a tail pointer you can change this to have a complexity of O(1). ie. this loop becomes unnecessary.

Node<T> * tmp = head;
{
tmp = tmp->next;
}


You have an output operator std::ostream & operator<<(std::ostream & os, const CircularLinkedList<U> & cll) It would be nice if you had a symmetric input operator. In this situation you need to worry about the size of the list (or adding a special terminating character). So the creation of the input operator will affect your design of the output operator.

-
I don't really see how smart pointers will help this code. In my mind, the most compelling reasons to use smart pointers are for exception safety and graceful cleanup. Here we have exactly 1 place where allocations happen. It's pretty manageable. I see the use of smart pointers here as a solution in need of a problem. Using smart pointer poorly would also make it worse, i.e. using shared_ptr would add gross amounts of overhead. –  asveikau Sep 8 '11 at 3:59

the code has problem when the list has only one node, and when it is removed, the head is not pointed to NULL.

e.g.,

int main() {

cout << cll << endl;
cll.deleteNode(5);
cout << cll << endl;
return 0;
}

-

Also, for simplicity I would have kept a reference to the last inserted item (calling it the tail might be wrong as it is circular, but then again is concise with your head). This way, inserting would be O(1) instead of O(n), which makes sense as you know you will add it last in the list, right before head. You could then do:

if (head == NULL) {