Help with Linked List code

[IronBat]

Hello I have written a linked list class but I would like someones opinion on it. If anyone would be so kind as to look at the code which follows and point out any ways I could improve it or any errors(I dont think there are any?)
I would REALLY appreciate it. Thanks in advance .

//-------------------------------------------------------------------------
// CNode
// NOTE: This class is the bones of the linked list. Each node contains the 
// data to be stored and a pointer to the next node in the list.
//-------------------------------------------------------------------------

template <class T>
class CNode
{
public:
  CNode();
  CNode(T *Data, int ID);
  ~CNode();
  Insert(T *Data, int ID);
  T *Get(int ID) const;
  void Remove(int ID);
  void UpDateID(int ID);
private:
  T *m_Data;
  int m_ID;
  CNode *m_Next;
  bool m_Delete;
};

// NOTE: This is just the defualt constructer.
template <class T>
CNode<T>::CNode()     
{
  m_Data = NULL;
  m_Next = NULL;
  m_Delete = true;
  m_ID = 0;
}

// NOTE: This is the constructer called to assigned data to the node.
template <class T>
CNode<T>::CNode(T *Data, int ID)
{
  m_Data = Data;
  m_Next = NULL;
  m_Delete = true;
  m_ID = ID;
}

// NOTE: This is the destructor, it only deletes the next node in the list if it is told to.
// Because I dont want the whole list to destroy if I only want to destroy one node.
template <class T>
CNode<T>::~CNode()
{
	delete m_Data;

	if(m_Delete == true) 
	{
		delete m_Next;
	}
}

// NOTE: This function inserts the data you pass to the list. First checks if this is the end 
// node, if it is it inserts a new node into the list. If it is not it passes the data on to 
// the next node.
template <class T>
CNode<T>::Insert(T *Data, int ID)
{
   if(m_Next == 0) 
   {
       m_Next = new CNode(Data, ID);
   }
   else 
   {
	   m_Next->Insert(Data, ID);
   }
}

// NOTE: This function will retrive the data you want. First it will check to see if it's ID 
// matches the number given. If it does it returns a pointer to the data you want otherwise 
// it qureeses the next node.
template <class T>
T* CNode<T>::Get(int ID) const
{
   if(ID == m_ID)
   {
	   return m_Data;
   }
   else
   {
       return m_Next->Get(ID);
   }
}

// NOTE: This function removes a node from the list. 
template <class T>
void CNode<T>::Remove(int ID) 
{
    if(m_ID == (ID - 1))
	{
	  CNode *temp = m_Next->m_Next; 
	  m_Next->Remove(ID);
	  m_Next = temp;
	}
	else if(m_ID == ID)
	{
	  m_Delete = false;
      m_Next->UpDateID(m_ID);
	  delete this; 
    }
	else
	{
	  m_Next->Remove(ID); // Pass it on
	}
}

// NOTE: This updates all the node ID's if a node has been removed
template <class T>
void CNode<T>::UpDateID(int ID)
{
	m_ID = ID;
    m_Next->UpDateID(ID++);
}


//-------------------------------------------------------------------------
// CList
// NOTE: This class is the brain of the linked list deals with allocating 
// data and keeping the total number of nodes in the list. 
//-------------------------------------------------------------------------

template <class T>
class CList
{
public:
    CList() {m_Next = 0; m_NodeCount = 0;}
	~CList(); 
	void Insert(T *Data);
	T *Get(int ID) const;
	void Remove(int ID);
	int GetNodeCount() const {return m_NodeCount;}
private:
    CNode<T> *m_Next;
	int m_NodeCount;
};

// Insert data to the list
template <class T>
void CList<T>::Insert(T *Data) 
{ 
	if(m_Next == 0) // If the first Node has not been made
	{
	    m_Next = new CNode<T>(Data, m_NodeCount++);
	}
	else
	{
		m_Next->Insert(Data, m_NodeCount++);
	}
}

// Get data from the list
template <class T>
T *CList<T>::Get(int ID) const
{ 
	return m_Next->Get(ID);
}

// Remove data from the list
template <class T>
void CList<T>::Remove(int ID)
{
	m_Next->Remove(ID);
	m_NodeCount--;
}

 

[Cagliostro]

I think you should use a debugger and to etst it yourself before. Ion Filipski

filipski@excite.com

 

[IronBat]

Well I have used it and so far I have had no problems I just wanted some tips on improving my code or any potential problems .

 

[Cagliostro]

some operators will be welcome:

template <class T>
class CList
{
...
int position;
public:
T* operator[](int ID){return Get(ID);}
T* operator++(){position++;....}//get next
T* operator--(){....}//get prev
operator T*(){Get(position);}
...
};
so you will use
CList<xxx> x;
....
xxx a = *x[100];
x++;
xxx b = x;
and my advice is to use objects instead fo pointers(T instead of T* when you get some element) Ion Filipski

filipski@excite.com

 

[IronBat]

Thank a lot thats the sort of help I wanted, I like the operator idea and will implement it.
I am not so sure about storing objects instead of pointers to objects as I need to be able to get a object from the list and alter it but the changes need to take effect on the object in the list too. I dont just want to get a copy of an object. So I need to store pointers right?

Example:

CList<CCat> CatList;

int Age = 3;
char Colour[] = &quot;Red&quot;

CCat *Cat1 = new CCat(Age, Colour);

CatList.Insert(Cat);

NewAge = ++Age;

CatList.Get(1)->SetAge(NewAge);

Int CatsAge = CatList.Get(1)->GetAge();

Or am I wrong?. Anyway thanks again.

 

[Cagliostro]

yes, but what are the references for? In C++ are enough possibilities to not using pointers. Ion Filipski

filipski@excite.com