I tried to do a simple factory but ended up with a wierd error?

Started by
24 comments, last by Servant of the Lord 11 years, 3 months ago

Hi,

I tried to do a basic hopefully almost correct factory where its simply just return new "insert class here".

Here the class I want to dublicate/create with my factory


class Enity
{
public:	
        Enity(){};
	std::vector<BaseComponent> ComponentList;
	template<class ComponentType>
	void AddComponent(ComponentType type);
	void Destroy();
};

Here the factory class, which Sit in a header btw


class BasicFactory
{
public:
	BasicFactory(){}
	virtual ~BasicFactory(){}

	template<class EnityType>
	EnityType* InstantiateEnity(EnityType type);
};
class EnityFactory : public BasicFactory
{
public:
	EnityFactory(){}
	virtual ~EnityFactory(){}
	template<class EnityType>
	Enity* InstantiateEnity(EnityType type);

};

And here I declared the function to the factory



template<class EnityType>
Enity* EnityFactory:: InstantiateEnity(EnityType type)
{
	return new type;
}

Seems like even a Idiot guide is to advance for me :(

http://www.codeproject.com/Articles/257589/An-Idiots-Guide-to-Cplusplus-Templates-Part-1

and here's the tutorial I kinda followed

Here what I tried to do


Enity TestComp;
std::vector<Enity> vEnity;
std::vector<Enity>::iterator vEnityEnd;
int vEnityMax = 10;

EnityFactory eFactory;
int vEnitySize = 0;





void Update()
{
	vEnitySize = vEnity.size();
	vEnityEnd = vEnity.end();
	if(vEnitySize < vEnityMax)
	{
		//MessageBox(0,"Lets hope this work","Enity have NOT Increased by 1 yet!",MB_OK);
TestComp = &eFactory.InstantiateEnity(Enity); <- error of illegal use of this type as an expression
		vEnity.insert(vEnityEnd,TestComp);
		MessageBox(0,"Screw This","Enity Increased by 1!",MB_OK);
	}

	Test.Update(pWCamera);
	TestB.Update(pWCamera);
}

Advertisement

  std::vector<BaseComponent> ComponentList;



I stopped reading here. You seem to be trying to have a container of elements of a base class, and then get polymorphic behavior on them. Well, that just can't be done. You need to have a container of pointers of some sort. std::vector<std::unique_ptr<BaseComponent>> is a good start, if you are using C++11.

Also, you seem to be missing a "t" in "Entity".

As far as I can see, you didn't actually post your error message. wink.png

As far as I can guess, your error is telling you that your template isn't correct. Here's your template:


template<class EnityType>
Enity* EnityFactory:: InstantiateEnity(EnityType type)
{
     return new type;
}

As far as I can throw an exception (Poor attempt at humor. It's late), here's what I think is wrong:

First, you return a type of 'Entity'. I think you meant, EntityType.

Second, 'new' creates a dynamic class of type 'Type', right? So 'new int' creates a new int. You can't do 'new 5', even though '5' is an instance of an int.

Your function doesn't create a new EntityType (a class type), it's trying to create a new instance of EntityType.

This is incorrect:


int myInt = 5;
new myInt;
//Or:
EntityType type;
new type;

This is correct:


new int;
//Or:
new EntityType;

Thus, this:


template<class EnityType>
Enity* EnityFactory:: InstantiateEnity(EnityType type)
{
     return new type;
}

Should be this:


template<class EnityType>
EnityType* EnityFactory:: InstantiateEnity(EnityType type)
{
     return new EnityType;
}

And since you aren't using the parameter at all, you might as well remove it:


template<class EnityType>
EnityType* EnityFactory:: InstantiateEnity()
{
     return new EnityType;
}
I didn't explain that too well, but did you understand what I mean? You can't 'new' an instance of a type, you can only new a type itself.
Edit: Now I see your error message!
Using the corrected code I show above, the correct way to call such a function would be:
Entity *myEntity = eFactory.InstantiateEnity<Entity>();
By the way, the definition of your entire templated function must be in the header file, as ugly as that is. It's a shortcoming of templates. Any function that is templated must be entirely (declaration and definition) in the header. sad.png

Thanks you guys for the help :3
I will try to fix both of the problems.

Sorry for asking again, I did the change as you mention Lord, but now it give me the error

error C2276: * : illegal operation on bound member function expression.

At the line you said the error is you are trying to assign an Entity the address of a method that returns a pointer to an Entity. I guess you would want to assign an Entity pointer (Entity *) the Entity pointer that is returned by the InstantiateEntity() method.

Hard to say without seeing the code. You probably missed some brackets. More fundamentally, factories are used when you need to create an object, but you don't know the type. Here you do know the type, and there is actually only one. If you did need a factory, it would not be templated like that because that implies you know the type. Try it without the factory, or at least without the templates. If you just want to copy an object, use the copy constructor.

>.<
I know why it gave me the * Error operator, I kinda failed to read correct (Was in a Hurry for my driving lesson) either way. tEntity is now a proper pointer and not a instance of Entity.

But May I ask
is this the correct way to insert something into a vector while am here I may aswell ask, instead of the * operator error I now get a error/warning on my template factory.
And the reason why I use template instead of pre defined classes is that I hope one day I will be good enough to make it more "dynamic" or more automatic instead of needing me to make a new factory for each new class.


std::vector vEntity;std::vector::iterator vEntityEnd;
int vEntityMax = 10;Entity* tEntity;
EntityFactory eFactory;
int vEntitySize = 0;
vEntitySize = vEntity.size();
vEntityEnd = vEntity.end();
if(vEntitySize < vEntityMax)
{
//MessageBox(0,"Lets hope this work","Entity have NOT Increased by 1 yet!",MB_OK); 
tEntity = eFactory.EntityInstantiate<Entity>();
vEntity.insert(vEntityEnd,tEntity);
MessageBox(0,"Screw This","Entity Increased by 1!",MB_OK);	
}


Edit:
Here's the error

Reason: cannot convert from 'Entity *' to 'const Entity'
1> No constructor could take the source type, or constructor overload resolution was ambiguous
1> c:\program files (x86)\microsoft visual studio 10.0\vc\include\xmemory(280) : see reference to function template instantiation 'void std::allocator<_Ty>::construct<Entity*&>(Entity *,_Other)' being compiled
1> with
1> [
1> _Ty=Entity,
1> _Other=Entity *&
1> ]
1> c:\program files (x86)\microsoft visual studio 10.0\vc\include\vector(668) : see reference to function template instantiation 'void std::_Cons_val<std::allocator<_Ty>,Entity,Entity*&>(_Alloc &,_Ty1 *,_Ty2)' being compiled
1> with
1> [
1> _Ty=Entity,
1> _Alloc=std::allocator<Entity>,
1> _Ty1=Entity,
1> _Ty2=Entity *&
1> ]
1> c:\program files (x86)\microsoft visual studio 10.0\vc\include\vector(688) : see reference to function template instantiation 'void std::vector<_Ty>::emplace_back<Entity*&>(_Valty)' being compiled
1> with
1> [
1> _Ty=Entity,
1> _Valty=Entity *&
1> ]
1> c:\program files (x86)\microsoft visual studio 10.0\vc\include\vector(675) : see reference to function template instantiation 'std::_Vector_iterator<_Myvec> std::vector<_Ty>::emplace<Entity*&>(std::_Vector_const_iterator<_Myvec>,_Valty)' being compiled
1> with
1> [
1> _Myvec=std::_Vector_val<Entity,std::allocator<Entity>>,
1> _Ty=Entity,
1> _Valty=Entity *&
1> ]
1> c:\users\conny.conny-dator\desktop\fusionempty\fusionempty\windowshandler.cpp(223) : see reference to function template instantiation 'std::_Vector_iterator<_Myvec> std::vector<_Ty>::insert<Entity*&>(std::_Vector_const_iterator<_Myvec>,_Valty)' being compiled
1> with
1> [
1> _Myvec=std::_Vector_val<Entity,std::allocator<Entity>>,
1> _Ty=Entity,
1> _Valty=Entity *&
1> ]
Vectors can't be declared "std::vector myVector;", a vector itself is a template, and must be declared like this:
std::vector<int> myVector; //If it's a vector of 'ints'

So yours would be:
std::vector<Entity> myVector; //Vector of Entities.

But polymorphism requires you to carry your entities in either a pointer or a reference.
std::vector<Entity*> myVector; //Vector of Entity pointers.

If you are inserting pointers into a vector, you do this:
Entity *myEntity = ...;
myVector.push_back(myEntity); //Adds it to the end of the vector.
Because the entity is already in pointer form, and the vector takes pointers, you pass it directly into it. No & or * or anything else.

Since you are creating the entities with 'new' you must delete them with 'delete' - and vectors won't automatically delete your dynamically allocated memory for you. This can get annoying.
//Loop over every element in the vector.
for(size_t i = 0; i < myVector.size(); i++)
{
     delete myVector[i]; //Unallocate the memory for this entity.
}
myVector.clear(); //Resize the vector to 0.
 
 
//In C++11 standard, we'd use:
for(Entity *entity : myVector)
{
     delete entity;
}
myVector.clear();

Vectors call the destructor of every object in it when the vector is destroyed. Pointers don't have destructors, and don't destruct the thing they point to (because what if two pointers are pointing at the same memory? Neither pointer knows about the other).
The vector destroys the pointers, but not the memory they point to.

Instead, if we want the destruction of the pointer to automatically clean up the memory (which would be cleaner and safer), we need to use 'smart pointers'.
Smart pointers are an idea for pointers that destroy what they point to, only if no other pointer is still using the memory.

In C++11 (if you have it available), you have std::unique_ptr, std::shared_ptr, and std::weak_ptr. You also have the defective std::auto_ptr (even before C++11), but don't use that, it's been deprecated for good reason.

So, you can have a vector of smart pointers. When the vector is destroyed, it destroys the smart pointers, and the smart pointers destroy the objects they point to.
std::vector<std::shared_ptr<Entity>> myEntities;


Now your template function needs to return a smart pointer instead of a regular pointer:
template<class EntityType>
std::shared_ptr<EntityType> EntityFactory:: InstantiateEntity()
{
     return std::make_shared<EntityType>();
}

std::make_shared<Type>() creates a std::shared_ptr pointing to a instance of 'Type' that std::make_shared dynamically allocates for you (so no need to call 'new'). Zero or more arguments can be passed to std::make_shared, and are handed to Type's constructor.

Thanks again for another awesomeness explanation,

P.S

Pointers everywhere!

Miss the simplicity of c# :(.

Performance question

If this vector that hold my pointers were to be "in game" enemies, would it be better to NOT delete it and simply "re use" it, after the "death timer"/"spawn timer" have been reach?. And is there a good website somewhere that would explain these types of detail? If I gonna start code in c++ I may aswell go right from the beginning >.<. oh would also be nice if anyone could give me a good place reading on about the

smart pointers seeing they seems to be good thing to know about. Untill then may my google-fu help me XD.

P.S

Everything works now! thanks alot

This topic is closed to new replies.

Advertisement