Jump to content
  • Advertisement
Sign in to follow this  
Grahf750

vector push_back with object

This topic is 4873 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 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.

Share this post


Link to post
Share on other sites
Advertisement
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

Share this post


Link to post
Share on other sites
if you're pushing the class on to the vector instead of a pointer you'll need a copy constructor.

Share this post


Link to post
Share on other sites
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

Share this post


Link to post
Share on other sites
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;
}

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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)

Share this post


Link to post
Share on other sites
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];
}

Share this post


Link to post
Share on other sites
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]

Share this post


Link to post
Share on other sites
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;
}

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!