My container classes

Started by
15 comments, last by MaulingMonkey 19 years, 7 months ago
Ok, I'm working on my game engine and I need a few container classes that are going to plug straight into it. The only two I have done are CResizeableArray and CString. I just wanted to see if I was missing any obvious memory leaks before I start using them. Please don't post saying I should use STL. I already know this and I'm using it everywhere else in my engine, just not the few places that these classes are designed specifically for (no arguing with me either, I'm using these wether you like it or not :D). Note that there are a few other functions to the classes that I have omitted for readability (I'm positive that there aren't any leaks in these).

template<class ContainerType>
class CResizeableArray
{
protected:
	ContainerType*	m_pArray;
	int				m_iArraySize;
	
public:
	CResizeableArray<ContainerType>(int iDefaultSize = 0)
	{
		m_pArray		= NULL;
		m_iArraySize	= 0;
		
		if(iDefaultSize > 1)
			ResizeArray(iDefaultSize);
	}
	
	
	~CResizeableArray<ContainerType>()
	{
		delete [] m_pArray;
	}
	
	
	ContainerType* ResizeArray(int iSize)
	{
		if(iSize > 1)
		{
			ContainerType* pBuffer = NULL;
			if(!(pBuffer = new ContainerType[iSize + m_iArraySize]))
				return NULL;
			
			if(m_pArray != NULL)
			{
				memcpy(pBuffer, m_pArray, m_iArraySize * sizeof(ContainerType));
				delete [] m_pArray;
			}
			
			m_pArray = pBuffer;
			m_iArraySize += iSize;
			
			return m_pArray;
		}
		
		return NULL;
	}
	
	
	void ClearArray()
	{
		delete [] m_pArray;
		m_iArraySize = 0;
	}
	
	
	ContainerType* GetArray() const
	{
		return m_pArray;
	}
	
	
	ContainerType GetArrayIndex(int iIndex) const
	{
		if(iIndex > -1 && iIndex < m_iArraySize)
			return m_pArray[iIndex];
	}
	
	
	ContainerType operator [](int iIndex) const
	{
		if(iIndex > -1 && iIndex < m_iArraySize)
			return m_pArray[iIndex];
		return NULL;
	}
	
	
	int GetSize() const
	{
		return m_iArraySize;
	}
};

class CString : public CResizeableArray<char>
{
public:
	CString()
	{
	}

	CString(char* pString)
	{
		ClearArray();
		ResizeArray(strlen(pString) + 1);
		strcpy(m_pArray, pString);
	}

	const CString& operator =(char* pString)
	{
		ClearArray();
		ResizeArray(strlen(pString) + 1);
		strcpy(m_pArray, pString);

		return *this;
	}

	const CString& operator =(string String)
	{
		ClearArray();
		ResizeArray(String.size());
		strcpy(m_pArray, String.c_str());

		return *this;
	}

	operator char*()
	{
		return GetArray();
	}

	char* GetString()
	{
		return GetArray();
	}
};


Thanks! [Edited by - Programmer16 on September 18, 2004 12:47:01 AM]
Advertisement
What happens if you create an array of size 1?
The memcpy you are using to copy your arrays when resizing is not a good idea if you plan on storing anything complicated. For example, if you store a bunch of objects that require a deep copy thus have overloaded the assignment and copy constructors they will not get called. If you plan to store only pointers in your dynamic arrays then you should be fine but I would check out the STL vector source or other dynamic array implementations. I guarentee you those memcpys will cause problems for you.
Protected member variables are a code smell.

Your string class shouldn't hit the heap for smaller strings.

CClassname is redundant. (FWIW)

A string isn't necessarily is-a type of resizable array. It should aggregate the resizable array.

Don't use strcpy for crying out loud! MSDN clearly states that incorrect usage of strcpy can trigger a buffer overflow!

The ResizeArray() member function doesn't make sense on a string.

Silent conversion operators can lead to ambiguities.

Use STL. If you dislike the notation, wrap the STL classes in your own and use those. But the use of strcpy() makes me question whether you should be using your own string class at this point, otherwise you're potentially introducing buffer overflows into your application when you get bad input.
--God has paid us the intolerable compliment of loving us, in the deepest, most tragic, most inexorable sense.- C.S. Lewis
pammat: Actually, the STL does the same thing. Calling the copy constructor followed by the destructor for the original object is not as efficient as memcpy, and in practice doesn't usually cause problems that aren't obvious.
Really?! I could have sworn I saw many calls to construct, destory and copy objects around.. hmm.. guess you learn something new everyday!
@ Sneftel:
Quote:Line from the constructor
if(iDefaultSize > 1)
ResizeArray(iDefaultSize);


@ antareus:
For some reason, I don't understand what this sentence means:
"A string isn't necessarily is-a type of resizable array. It
should aggregate the resizable array."

Do you mean it is a resizeable array or it isn't?
And (if I'm right in what aggregate means) you think that the CString class and CResizeable class should be the same?

I use the protected keyword to keep the data away from the users, but available to derived classes.

I know about the strcpy function, and I've been working on my own version of it to fix the problem.

I don't care if you don't like CClassname, but it's not redundant, it stands for Container :P. (What does FWIW mean?)

The ResizeArray() function can be used in the append function I'm going to add to it.

I already said that I'm using STL everywhere else, but I needed some extra functions and such added to it. First, I don't know how to derive from STL (I assume that it's probably like deriving from any other classes, but I don't know if I have to handle calls to certain behind the scene functions (like calling addref() in COM)). Second, did you guy's skip over the paragraph that says that I'm not going to use STL? I know STL is a better choice and all, but I'm still going to use these.

@ pammatt: I'm going to be using the CString class with my font, scripting, and console class. The CResizeableArray class is going to be used in part of my graphics system.
In resize you have a delete m_pArray, instead of delete[] m_pArray.
Your string class should hold a copy of a resizable array inside of it to use as the buffer. It should not derive from the resizable array class. When you derive from a class you are saying "a string IS-A resizable array in the same way that a lion IS-AN animal." If I declare a string, it doesn't make any sense for me to be able to call a member function named "ResizeArray()" on a string. That is unnecessarily exposing implementation details. Private inheritance would be more appropriate in this situation.

I recommended you use STL because your use of strcpy() is questionable, as well as boundary conditions (allocating an array of one element) being handled and wrong delete operator being used. You wouldn't derive from an STL class but instead just have your class contain the STL container and then you'd implement whatever interface you want over it. I can understand your frustration with the external interface presented by STL - it is definitely pretty low-level and cryptic at times. But it is also tested, speedy code.
--God has paid us the intolerable compliment of loving us, in the deepest, most tragic, most inexorable sense.- C.S. Lewis
Here, I've rewritten your array class for you. Rewriting the string class is left as an exercise to the reader.

template<class ContainerType>class CResizeableArray{private:    std::vector<ContainerType> v;	public:    CResizeableArray<ContainerType>(int iDefaultSize = 0) :        v(iDefaultSize)    { }		    ContainerType* ResizeArray(int iSize)    {        v.resize(iSize);    }    void ClearArray()    {        v.clear();    }		    ContainerType* GetArray() const    {        return v.empty() ? 0 : &v[0];    }    ContainerType GetArrayIndex(int iIndex) const    {        return v.at(iIndex);    }		    ContainerType operator [](int iIndex) const    {        return v.at(iIndex);    }		    int GetSize() const    {        return v.size();    }};

This topic is closed to new replies.

Advertisement