Code Validation

[bsHobbit]

Didnt know any better Title ;)

I just wanted to know if the following code is bad or good or neither of both.

And if it's bad what could I improve?

Thanks for any advice :)

template <class T>
class RList
{
	private:
		//Structure that contains all information of the list
		struct Data
		{
			T Value; //Value of the Element
			Data *pNext; //Pointer to next Value
		};

		
		Data *pFirst; //Pointer to the first element of the list
		Data *pTail; //Pointer to the last element of the list
		int TotalObjects; //To save Count() performance
	public:
	
		//[] Operator implementation for the list
		T& operator[] (int index)
		{
			if (index < 0 || index >= Count())
				throw "Index out of range";

			Data *p = pFirst;
			
			for (int i = 0; i < index; ++i)
				p = p->pNext;

			return p->Value;
		}

		//Initialize the list
		RList()
		{
			pFirst = NULL;
			pTail = NULL;
			TotalObjects = 0;
		}

		//Remove all list elements
		~RList()
		{
			Clear();
		}

		//Add an element to the list
		void Add(T value)
		{
			Data* pNew = (Data*)malloc(sizeof(Data));
			pNew->Value = value;
			pNew->pNext = NULL;

			if (pFirst == NULL) {
				pFirst = pNew;
				pTail = pNew;
			} else {
				pTail->pNext = pNew;
				pTail = pNew;
			}

			TotalObjects++;
		}

		//Insert an element at any position in the list
		void InsertAt(T Value, int index)
		{
			if (index < 0 || index >= Count())
				throw "Index out of range";
			
			Data* pNew = (Data*)malloc(sizeof(Data));
			pNew->Value = Value;
			if (index == 0) {
				pNew->pNext = pFirst;
				pFirst = pNew;
			} else {
				Data *prev = pFirst;
				Data *p = pFirst;
				for (int i = 0; i < index; ++i) {
					prev = p;
					p = p->pNext; 
				}
				prev->pNext = pNew;
				pNew->pNext = p;
			}

			TotalObjects++;
		}

		//Counts all objects in the list
		int Count()
		{
			return TotalObjects;
		}

		//Removes an element from the list
		void RemoveAt(int index)
		{
			if (index < 0 || index >= Count())
				throw "Index out of range";

			if (index == 0) {
				Data *p = pFirst->pNext;
				
				if (pFirst == pTail)
					pTail = p;

				free (pFirst);
				pFirst = p;
			} else {
				Data *prev = pFirst;
				Data *p = pFirst;
				Data *pNext = pFirst->pNext;
				for (int i = 0; i < index; ++i) {
					prev = p;
					p = p->pNext;
					pNext = p->pNext;
				}
				if (p == pTail)
					pTail = prev;
				free (p);
				prev->pNext = pNext;
			}

			TotalObjects--;
		}

		//Clear all Objects in List
		void Clear()
		{
			while (pFirst)
				RemoveAt(0);
		}

		//Checks if a value is part of the list
		bool Contains(T Value)
		{
			Data *p = pFirst;
			while (p != NULL)
			{
				if (p->Value == Value)
					return true;
				p = p->pNext;
			}
			return false;
		}
};

[zb0S216C]

After a quick glance, I can see three issues:

1) I would change your default-constructor to the following because you're not initialising, your assigning:

RList::RList( )
  : pFirst( NULL ), pTail( NULL ), TotalObjects( 0u )
{ }

2) Also, I would move all of the function definitions within your class to the outside of your class. For example:

template< typename T >
class RList
{
  public:
    void SampleFunction( );
};

template< typename T >
void RList< T >::SampleFunction( )
{
}

This makes your code easier to read and does not implicitly mark your functions as in-line.

3) You're not checking for a failed allocation after each call to "malloc( )".

_Wazzak