Sign in to follow this  

Template classes as class members

This topic is 3725 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I made a Dynamic Array (DynArray) template class, and an Image class. I also made a sprite class. To combine them, I made a DynamicArray of Image objects which was a member var of my Sprite class So, its something like this Header
class Sprite
{
public:
	/*template <class T>*/  Sprite();
	//template <class T>  Sprite(Image);


	//Doers
	template <class T>  void Draw();

	//Setters
	template <class T>  void SetImage(int);
	template <class T>  int AddImage(Image);
	template <class T>  void RemoveImage(int ind);
	template <class T>  void SetPos(int x, int y);
	
	//Getters
	template <class T>  SDL_Surface* GetSDL(int ind);
	template <class T>  GLuint		 GetOGL(int ind);
	template <class T>  int	GetWidth();
	template <class T>  int GetHeight();
	template <class T>  int GetCurrentImage();



private:
	DynArray<Image> Im;
	int x,y,h,w,CurrentImage;
	
};

etc Source
sing namespace Gfx;
using namespace Log;


template <class T> Sprite::Sprite() : DynArray<Image> Im
{
	CurrentImage=-1;
	x=0;
	y=0;
}



// / / / / / / / / / / / / / / / / / / / /
//Func: Sprite::Draw()
//
//Purpose: Draw the sprite
//
//Returns: 
// / / / / / / / / / / / / / / / / / / / /
template <class T> void Sprite::Draw()
{
	//Make sure we have a current image, and check its integrity
	if (CurrentImage>=0 && CurrentImage<=Im.UpperB())
	{
		//Use Image class native draw func()
		Im[CurrentImage].SetPos(x,y);
		Im[CurrentImage].Draw();
	}
	else
	{
		WriteLine(Warning,"Attempted to draw out-of-bounds element at Sprite::Draw() ");
	}
}



// / / / / / / / / / / / / / / / / / / / /
//Func: Sprite::AddImage()
//
//Purpose: Add an already loaded SDL surface into our array of images
//
//Returns: Index of the Image in the array
// / / / / / / / / / / / / / / / / / / / /
template <class T> int Sprite::AddImage(Image temp_im)
{
	//Add the Image to the array
	Im.Add(temp_im);
	//As we added it to the array, it will be the last
	//element (or the upper bound)
	return Im.UpperB();
}



// / / / / / / / / / / / / / / / / / / / /
//Func: Sprite::SetImage()
//
//Purpose: Set image that the sprite displays
//
//Returns:
// / / / / / / / / / / / / / / / / / / / /
template <class T> void Sprite::SetImage(int num)
{
		//Greater than zero, less or equal to the UpperBound (last element)
		if (num>=0 && num<=Im.UpperB())
		{
			CurrentImage=num;
		}
		else
		{
			//Log the error if there is one
			WriteLine(Warning,"Attempted to set Sprite's Current Image to an out-of-bounds element: ("+its(num)+")");
		}
	
}


template <class T>  SDL_Surface* Sprite::GetSDL(int num)
{
	//Make sure we arent referencing a null pointer
	if (num<=Im.UpperB() && num>=0)
	{
		if ( Im[num].GetSDL() != NULL)
		{
			return Im[num].GetSDL();
		}
		else
		{
			//Log the error if there is one
			WriteLine(Warning, "Attempted to retrieve a NULL pointer @ Sprite::GetSDL("+its(num)+")");
		}
	}

	return NULL;
}

template <class T> int Sprite::GetCurrentImage()
{
	return CurrentImage;
}

template <class T> int Sprite::GetWidth()
{
	return Im[CurrentImage].Width();
}

template <class T> int Sprite::GetHeight()
{


	return Im[CurrentImage].Height();
}



But when I try to instantiate a Sprite object, the compiler complains of "No defaul constructor for Sprite::Sprite" and there are troubles with the DynArray<Image>. I can use the Image classes member funcs, but anything like Adding elements to the array and deleting them I cant seem to do Any help?

Share this post


Link to post
Share on other sites
To use a template as a member your class does not need to be templated.

This should work:

class Sprite
{
public:
Sprite();
Sprite(Image);


//Doers
void Draw();

//Setters
void SetImage(int);
int AddImage(Image);
void RemoveImage(int ind);
void SetPos(int x, int y);

//Getters
SDL_Surface* GetSDL(int ind);
GLuint GetOGL(int ind);
int GetWidth();
int GetHeight();
int GetCurrentImage();



private:
DynArray<Image> Im;
int x,y,h,w,CurrentImage;

};




Next up, once you have completed DynArray to your satisfaction, throw it away. While it is worth implementing for learning purposes, the Standard C++ Library already has one, std::vector<T>. Learn to use it, and its sibling container classes.

Finally, your class appears to have very little behaviour, but a lot of setters and getters. This is (in general) indicative of poor design.

Share this post


Link to post
Share on other sites
All of you "template <class T>" are redundant, or at least don't do what you want them to, but definitely do nothing, except perhaps increase code bloat.

What exactly are you trying to achieve with these templates, since it's not clear.

What would the T template argument be in case of Sprite?

Share this post


Link to post
Share on other sites
1>sprite.obj : error LNK2019: unresolved external symbol "public: __thiscall DynArray<class Image>::DynArray<class Image>(void)" (??0?$DynArray@VImage@@@@QAE@XZ) referenced in function "public: __thiscall Sprite::Sprite(void)" (??0Sprite@@QAE@XZ)
1>sprite.obj : error LNK2019: unresolved external symbol "public: int __thiscall DynArray<class Image>::UpperB(void)" (?UpperB@?$DynArray@VImage@@@@QAEHXZ) referenced in function "public: int __thiscall Sprite::AddImage(class Image)" (?AddImage@Sprite@@QAEHVImage@@@Z)
1>sprite.obj : error LNK2019: unresolved external symbol "public: void __thiscall DynArray<class Image>::Add(class Image)" (?Add@?$DynArray@VImage@@@@QAEXVImage@@@Z) referenced in function "public: int __thiscall Sprite::AddImage(class Image)" (?AddImage@Sprite@@QAEHVImage@@@Z)
1>C:\Documents and Settings\Conor Haddock\My Documents\Visual Studio 2005\Projects\KittyKat\Debug\KittyKat.exe : fatal error LNK1120: 3 unresolved externals



How do I remedy this?

My sprite class would have Image as the template parameter

Share this post


Link to post
Share on other sites
Quote:
Original post by ConorH
*** Source Snippet Removed ***

How do I remedy this?


Current compilers need to be able to see a template classes definition to link work. If you have a dynarray.cpp (or similar), move the code there into dynarray.h. If not, maybe you forgot to define these functions?

If all else fails, post your dynarray. In fact, it might be a good idea anyway, properly implementing such a data structure is very difficult. It might do your understanding no end of good to have your code peer reviewed here.

Share this post


Link to post
Share on other sites
DynArray.h

#include <string>

template <class T>
class DynArray
{
public:
//Ctors
DynArray();
DynArray(int); //Specify size

//Dtors
~DynArray();

//Member funcs
void Add(T);
void Insert(int ind, T val);
void Remove(int ind);
void Empty();
void Set(int ind,T val);
int Find(int ind);
int FindNext(int ind);

//Setters
void SetSize(int sz);

//Getters
int GetSize();
int UpperB(); //Get upper bound, or the index of the last element (Lower Bound is 0)

//Operators
T& operator[](const int& index)
{return pArray[index];}

private:
int Size,LastFind;
T* pArray;
};



DynArray.cpp


// / / / / / / / / / / / / / / / / / / / /
//Func: template <class T> DynArray<T>::Ctor()
//
//Purpose: Create our initial array
//
//Returns:
// / / / / / / / / / / / / / / / / / / / /
template <class T> DynArray<T>::DynArray()
{
Size=0;
T* pArray = new T[Size];
}

// / / / / / / / / / / / / / / / / / / / /
//Func: template <class T> DynArray<T>::Ctor()
//
//Purpose: Create our initial array, to s specified size
//
//Returns:
// / / / / / / / / / / / / / / / / / / / /
template <class T> DynArray<T>::DynArray(int sz)
{
Size=sz;
T* pArray = new T[Size];
}

// / / / / / / / / / / / / / / / / / / / /
//Func: template <class T> DynArray<T>::Ctor()
//
//Purpose: delete array and clean up
//
//
//Returns:
// / / / / / / / / / / / / / / / / / / / /
template <class T> DynArray<T>::~DynArray()
{
delete [] pArray;
}

// / / / / / / / / / / / / / / / / / / / /
//Func: template <class T> DynArray<T>::Add()
//
//Purpose: Add an element to the end of an array
//
//Returns: false if fail
// / / / / / / / / / / / / / / / / / / / /
template <class T> void DynArray<T>::Add(T x)
{
//Inc the size of the array to account for the new element
Size++;

//Make the new array
T* pTempArray = new T[Size];

//Copy the old memory into it
memcpy(pTempArray,pArray,(Size-1)*sizeof(T));

//Assign the value to the last element
pTempArray[Size-1] = x;

//Delete the original array
delete [] pArray;

//Set the classes pointer to the newly created memory. pTempArray
//will only be in memory inside this function, so we dont have to
//delete it
pArray=pTempArray;
}


// / / / / / / / / / / / / / / / / / / / /
//Func: template <class T> DynArray<T>::Insert()
//
//Purpose: Insert an element at a specified index
//
//Returns: false if fail
// / / / / / / / / / / / / / / / / / / / /
template <class T> void DynArray<T>::Insert(int index, T x)
{
//Make a var that stores the index in the array (considering a lower bound of 0 rather than 1)
int TrueIndex=index+1;

//Increment for the new sized array
Size++;

//Make our new sotring array
T* pTempArray = new T[Size];

//Copy the first half (index-1) to the new array from the old
memcpy(pTempArray,pArray,index*sizeof(T));

//then the second half (index) from source to (index+1) in dest
memcpy(pTempArray+TrueIndex,pArray+TrueIndex-1,(Size-index)*sizeof(T));

//Apply the new value to the array
pTempArray[index]=x;

delete [] pArray;

pArray=pTempArray;
}


// / / / / / / / / / / / / / / / / / / / /
//Func: template <class T> DynArray<T>::Remove()
//
//Purpose: Remove an element and resize the array
//
//Returns: false if fail
// / / / / / / / / / / / / / / / / / / / /
template <class T> void DynArray<T>::Remove(int index)
{
//Make a var that stores the index in the array (considering a lower bound of 0 rather than 1)
int TrueIndex=index+1;

//Increment for the new sized array
Size--;

//Make our new sotring array
T* pTempArray = new T[Size];

//Copy the first half (index-1) to the new array from the old
memcpy(pTempArray,pArray,index*sizeof(T));

//then the second half (index) from source to (index+1) in dest
memcpy(pTempArray+TrueIndex,pArray+TrueIndex+1,(Size-index)*sizeof(T));

//Delete our old array
delete [] pArray;

//point at the new one
pArray=pTempArray;
}

// / / / / / / / / / / / / / / / / / / / /
//Func: template <class T> DynArray<T>::Empty()
//
//Purpose: Set all values to 0
//
//Returns:
// / / / / / / / / / / / / / / / / / / / /
template <class T> void DynArray<T>::Empty()
{
for (int i = 0; i < Size; i++)
{
pArray[i]=0;
}
}

// / / / / / / / / / / / / / / / / / / / /
//Func: template <class T> DynArray<T>::Set()
//
//Purpose: Set an element to a value
//
//Returns:
// / / / / / / / / / / / / / / / / / / / /
template <class T> void DynArray<T>::Set(int ind, T val)
{
//Make sure we are accessing an element in bounds
if (ind>=0 && ind<=UpperB())
{
pArray[ind]=val;
}
}

// / / / / / / / / / / / / / / / / / / / /
//Func: template <class T> DynArray<T>::GetSize()
//
//Purpose:
//
//Returns: Total number of elements
// / / / / / / / / / / / / / / / / / / / /
template <class T> int DynArray<T>::GetSize()
{
return Size;
}

// / / / / / / / / / / / / / / / / / / / /
//Func: template <class T> DynArray<T>::UpperB()
//
//Purpose:
//
//Returns:Last element's index
// / / / / / / / / / / / / / / / / / / / /
template <class T> int DynArray<T>::UpperB()
{
return Size-1;
}

Share this post


Link to post
Share on other sites
I agree that you would be better off using std::vector instead of DynArray. It will be much faster because it won't allocate memory and move things around each time you add something. And you really shouldn't be moving unknown objects around with memcpy - they need to be properly copied using assignment operator/copy constructor.

Share this post


Link to post
Share on other sites
Quote:
Original post by ConorH
DynArray.h
*** Source Snippet Removed ***


DynArray.cpp
*** Source Snippet Removed ***


You class is flawed in many places. I'm going to review it line by line, for the moment try this:


// place this all in dynarray.h
#include <vector>

template <class T>
class DynArray
{
public:
//Ctors
DynArray(){}
DynArray(int i):vec(i){}

//Dtors
//~DynArray();

//Member funcs
void Add(const T &t) { vec.push_back(t); }
void Insert(int ind, const T &val) { vec.insert(vec.begin() + ind, val); }
void Remove(int ind) { vec.erase(vec.begin() + ind ); }
void Empty() { vec.clear(); }
void Set(int ind,const T &val) { vec.at(ind) = val; }

/* removed due to unclear functionality
int Find(int ind);
int FindNext(int ind);
*/

//Setters
void SetSize(int sz) { vec.resize(sz); }

//Getters
int GetSize() const { return vec.size(); }
int UpperB() const { return GetSize() - 1; }

//Operators
T& operator[](int index)
{return vec[index];}

const T& operator[] const(int index)
{return vec[index];}

private:
std::vector<T> vec;
};


As you can see std::vector neatly supports all the operations you want to use. You should consider using it directly in your sprite class.

Let us consider some of the things that are wrong with your class.

1) memcpy. Don't use memcpy, it cannot work on non-POD objects. Always use std::copy.

2) When you add objects to the array, it reallocates the internal array every time. This is extremely costly. Consider a strategy like std::vector, overallocation.

3) Empty(). This makes no sense? Why have a function to set all the values to 0?
I was expecting something similar to either bool std::vector::empty() const; which tells me if Size == 0, or void std::vector::clear(); which would clear all the elements from the dynamic array.

4) Perhaps Set() should signal the out of bounds access as an error (possibly via an assert, or an exception) rather than silently ignoring it.

5) GetSize() and GetUpperB() should be const.

6) You should provide "const T &operator[]( int ) const;" too.

7) No copy constructor or assignment operator. Follow the "rule of three". If you need any of {destructor,copy constructor, operator=}, then you probably need all 3.

Share this post


Link to post
Share on other sites
I took the example of my DynArray from Practical C++, so I thought it would be half decent, though I did wonder whether there was a more effecient way to insert values into arrays than just copying either half. I will use std::vector rather than my own class from now on.

Share this post


Link to post
Share on other sites
Quote:
Original post by ConorH
I took the example of my DynArray from Practical C++, so I thought it would be half decent, though I did wonder whether there was a more effecient way to insert values into arrays than just copying either half. I will use std::vector rather than my own class from now on.


A quick Google search reveals a few different books titled "Practical C++", with widely varying copyright dates. Regardless, *currently*, implementing this kind of thing yourself is very impractical C++ :) That said, there's something to be gained from learning how these things work; it's best not to think too much about them while using them, though (only do enough thinking-about-implementation to decide which container you really want).

Share this post


Link to post
Share on other sites

This topic is 3725 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this