I have a vector that stores the different lifeforms in a battle. My function that adds a lifeform to the vector does not work. I do not understand why. I have put objects in vectors the same way before and so I do not understand why this does not work. In the Battle class header file: public: void addBattleChar(Lifeform lifeform); private: vector<Lifeform> battleChars; in the battle class source file: void Battle::addBattleChar(Lifeform lifeform) { battleChars.push_back(lifeform); } I get the error: :\Program Files\Microsoft Visual Studio .NET 2003\Vc7\include\xutility(1136) : error C2679: binary '=' : no operator found which takes a right-hand operand of type 'const Lifeform' (or there is no acceptable conversion) I am not sure if this is enough info I really have no idea what is wrong.

What you have to do here is add an operator overload of = in your Lifeform class definition.

add:

Lifeform& operator=( a_Lifeform& rhs )
{
*this = rhs;
return *this;
}

as a Private: member

ace

I'm not sure that is always true, if you will settle for only shallow copying then the default behaviour is fine.

I only ever have containers containing points to objects.

ace

I do have an overloaded = operator the code is this:

header:

Lifeform& operator=(Lifeform& lifeform);

source:

Lifeform& Lifeform::operator=(Lifeform& lifeform)
{
this->setName(lifeform.getName());
this->stats = lifeform.stats;
this->skills = lifeform.skills;
return *this;
}

Quote:Original post by ace_lovegrove
What you have to do here is add an operator overload of = in your Lifeform class definition.

add:

Lifeform& operator=( a_Lifeform& rhs )
{
*this = rhs;
return *this;
}

as a Private: member

ace

1. syntactically ok but semantically wrong, assignment operators should take constant reference as parameters.

2. That code will go into an infinite recursion [grin].

3. In his case it appears the types are the same, he does not need to explicitly define an assignment operator as one is implicitly defined to do member-wise assignement/copy.

4. push_back typically does an in-place copy construction not assignment.

Quote:Original post by gumpy macdrunken
you'll need a copy constructor.

He doesn't need to as one is implicitly defined by default to do a member-wise copy.

Quote:Original post by Grahf750
I do have an overloaded = operator the code is this:

header:

Lifeform& operator=(Lifeform& lifeform);

that is not the proper form of an assignment operator its meant to be:

Lifeform& operator=(const Lifeform& lifeform);

in any case as i mentioned previously you do not need to explicitly define an assignment operator or copy constructor as one is implicitly defined (compiler generated) be default to do a member-wise copy/assign.

Quote:Original post by Grahf750
void Battle::addBattleChar(Lifeform lifeform)

void Battle::addBattleChar(const Lifeform& lifeform)

I would suggest dynamically allocating the Lifeform instances, as passing them as pointers will save some memory and save you the trouble of using copy constructors.

-----------------------------------------------------
public:
void addBattleChar(Lifeform *const lifeform);

private:
vector<Lifeform*> battleChars;

void Battle::addBattleChar(Lifeform *const lifeform)
{
battleChars.push_back(lifeform);
}
-----------------------------------------------------

Then to access them simply use ... battleChars[index]->f();

Remember though, in your Battle class destructor you should delete all the Lifeform instances in the battleChar vector.

for (unsigned int n = 0; n < battleChars.size(); n++) {
delete battleChars[n];
}

Quote:Original post by AsOne
I would suggest dynamically allocating the Lifeform instances, as passing them as pointers will save some memory and save you the trouble of using copy constructors.

I doubt you will save memory, an array of pointers to "Lifeform" + all the individual instances of "Lifeform" allocated on the heap VS just an array of Lifeforms on the heap.

Plus if you know what your doing you can make sure that only one copy is ever made besides the cost of copy construction and destruction maybe insigificant/negligible compared to the cost of allocation/deallocation. Thankfully all standard library containers are parameterized by allocator type so you can use custom memory model/schemes with it.

Also std::vector separates construction/destruction & allocation/deallocation for efficiency. Typically growing is efficient to but when growing does eventually occur internal copying may happen (you can avoid the auto reallocation scheme using std::vector::reserve but you may never know how many elements to reserve) so if you happen to be growing alot you should consider std::deque which is more efficent at growing than std::vector, but std::vector is guaranteed to store elements contiguously in memory which maybe important.

Whether the copy constructor is really going to hurt performance needs to be profiled, pointers should only be stored when necessary it also depends on the context aswell in some cases you have no choice but to store them, read this thread for more interesting stuff on the topic.

[Edited by - snk_kid on June 15, 2005 7:34:04 PM]

I tried the const thing but now in this piece of code I get:

Lifeform_M.cpp(148) : error C2662: 'Lifeform::getName' : cannot convert 'this' pointer from 'const Lifeform' to 'Lifeform &'
Conversion loses qualifiers

Lifeform& Lifeform::operator=(Lifeform& lifeform)
{
this->setName(lifeform.getName());
this->stats = lifeform.stats;
this->skills = lifeform.skills;
return *this;
}