Couple of questions about objects

[georgep]

In a simple SDL 'game', the player picks up 5 coins to win. The coin is 'picked up' when the player touches it and a new coin is spawned in another random location.

However, in the code, when the player picks up the item, the item doesn't disappear; it's position is simply moved. (I found it much easier to do it this way)

if(player.x + player.width > item.x
   && player.x < item.x + item.width
   && player.y + player.height > item.y
   && player.y < item.y + item.height)
  {
    score++;
    item.x = random(0, SCREEN_WIDTH - item.width);
    item.y = random(0, SCREEN_HEIGHT - item.height);
  }

[I should add that random() is my own function that simply uses rand to generate a number in a given range]

Q1: Is this the way I should do it? I feel I would get more practice if instead I deleted the object when it is 'picked up' and created a new one. (The idea that the object just 'moves' also feels wrong to me.) If this is a better option, how would I do this?

___________________________________________________________________________

In another simple game, enemies can spawn and be killed. The way I am doing this is by using a vector of the enemy objects, and adding a new object when a new enemy spawns. (At the moment, this is not implemented, but I have tested using objects in this way).

class Zombie
{
public:
	Zombie(int a=100);
	int health;

};

Zombie::Zombie(int a)
{
	health = a;
	cout << "zombie spawned with " << health << " health\n";
}

class Horde
{
public:
	Horde(int spaces = 1);
	void Add(const Zombie& zombie);
	void Display();
	void Remove(const Zombie& zombie);

	vector<Zombie> zombies;
	
};

Horde::Horde(int spaces)
{
	zombies.reserve(spaces);
}

void Horde::Add(const Zombie& zombie)
{
	zombies.push_back(zombie);
}

void Horde::Display()
{
	for(vector<Zombie>::const_iterator iter = zombies.begin();
			iter != zombies.end();
			++iter)
	{
		cout << iter->health << " health\n";
	}
}

[This code shows how I am using a vector of objects - in this case, zombies - and how I am iterating through them. (It also acts as a way of showing how terrible I am at using vectors)]

Q2: How would I remove a specific enemy from the vector? I know erase is very inefficient, but it seems most suitable in this case. And how do I refer to this specific object in the vector? I will know which enemy needs to be removed, because I will iterate through the vector and check for collisions with each tick. However, I don't know how I use this erase an object from the vector.

[coder777]

Q1: Making a copy and move the item is a good idea because new/delete is costly and fragments the memory

Q2: Use list instead. if you replace vector with list the code below doesn't have to be changed.

erase removes an iterator
http://www.cplusplus.com/reference/stl/list/erase/

while remove removes a value
http://www.cplusplus.com/reference/stl/list/remove/

[georgep]

Okay, thanks for help.

Q1: So you mean keep it as it is? Didn't quite understand what you meant by copy the item.

Q2: Ehh, I went ahead and tried using a list, but I've never used them before and I got a bunch of errors. Fiddled with the code a bit and here's what I came up with:

void Horde::Remove()
{
  
    for(vector<Zombie>::iterator iter = zombies.begin(); iter < zombies.end(); ++iter)
    {
      if(iter->health <= 0)	
      {
        iter = zombies.erase(iter) - 1;
	cout << "zombie killed\n******\n";
      }
    }
}

[Peter87]

Q1: I don't see a problem with the way you have it. If you only have one item that is not dynamically allocated there is no way to remove the item anyway. Well, you could call the destructor explicitly and then use placement new to create the new item but that feels overkill.

Q2: If the hordes only contains a few zombies you will probably not be able to notice if erase is slow or not.

I'm a bit puzzled by coder777's recommendation. First he says that new/delete is costly and fragments the memory and then he suggest using std::list that has to use new/delete for every new zombie that is added/removed.

The choice of container is not always easy. Iterating through an std::list is slower than iterating through std::vector but deleting from std::list is often faster than std::vector so it depends on how often you do these things.

erase returns the iterator to the element after the erased element. Your code is not safe because if the first element in the vector is removed iter will point to the element before the first element which is not allowed. The way you should do it is to give iter the value that erase returns and only call ++iter; if erase was not called.

for(vector<Zombie>::iterator iter = zombies.begin(); iter < zombies.end();)
{
	if(iter->health <= 0)	
	{
		iter = zombies.erase(iter);
		cout << "zombie killed\n******\n";
	}
	else
	{
		++iter
	}
}

If the order of the zombies is not important you can erase from a vector much faster by move/copy the last element to the element that should be erased and then remove the last element.

for(vector<Zombie>::iterator iter = zombies.begin(); iter < zombies.end();)
{
	if(iter->health <= 0)	
	{
		*iter = std::move(zombies.back());
		zombies.pop_back();
		cout << "zombie killed\n******\n";
	}
	else
	{
		++iter
	}
}

If you are not using C++11, change line 5 to *iter = zombies.back();. This will copy instead of move which is sometimes slower.

[georgep]

@Peter:

Q1: Okay, will keep it as it is. Thanks.

Q2: Good point about the first element problem. Will change the code.