Sorry, another vector question 2

[fugigoose]

These vectors are kicking my butt! 'layer1' is defined as such:

vector <cls_sprite*> layer1;

I'm adding sprites to it with my addSprite method:

cls_sprite* cls_spriteCollection::addSprite(int layer){

if (layer == 1){
layer1.push_back(new cls_sprite());
layer1[layer1.size() - 1]->id = layer1.size() - 1;
return layer1[layer1.size() - 1];
}

}

Here's my 'removeSprite' method. It's function is to delete a sprite and the element in the vector 'layer1' pointing to it.

void cls_spriteCollection::removeSprite(int layer, int id){

if (layer == 1){
delete [] layer1[id];
layer1.erase(layer1.begin() + id);
if (layer1.size() > 0){
for (int i = 0; i == layer1.size() - 1; i++){
layer1[i]->id = i;
}
}
}

}

The problem is with this function. It generates all sorts of strange access and exception errors, the bad ones with the msg box that says 'abort, retry, fail'. What am i doing wrong? Thanx, maybe someday i'll quit asking vector questions.

 

[Salem]

> delete [] layer1[id];
Should be
delete layer1[id];
you only created a single object in the preceding new statement, not an array of them.

> for (int i = 0; i == layer1.size() - 1; i++)
This seems wrong.
If layer1.size() is anything other than 1, then the loop never happens
Even if it is 1, the loop only happens once.

--

 

[uolj]

You used new to create a single object but you used delete [] as if you were deleting an array. Use delete without the [].

 

[globos]

Just a little hint that will make body of addSprite cleaner : use 'back' function instead of 'operator[size - 1]' to access the last element of a vector:

cls_sprite* cls_spriteCollection::addSprite(int layer)
{
  if (layer == 1)
  {
    layer1.push_back(new cls_sprite());
    layer1.back ()->id = layer1.size() - 1;
    return layer1.back ();
  }

  return 0;
}

Another thing, in your removeSprite procedure, you could start from index 'id' to refresh the indexes :

for (int i = id; i < layer1.size(); i++)
  layer1[i]->id = i; //to be tested

--
Globos

 

[uolj]

Also a note, erasing elements from the middle of a vector can be an expensive operation (like O). Unless you are using the vector to be able to pass a C-style array to legacy code, you might want to consider other STL containers if you erase a lot. Even a deque, which is similar to a vector, might provide slightly better overall performance.

 

[fugigoose]

Thank you guys. I was a little confused about the delete and for syntax it seems. I was under the impression that the [] just ensured the object's deconstructor was called, but I guess not. Is a for statement nothing more than a while loop that can handle a variable? Like does it run just until the second parameter isn't true? I'm really not used to c++ for staements.

 

[uolj]

You use delete [] whenever you use new []. In other words, if you use new to allocate space for an array, then use delete []:

int* array = new int[100]; // array of 100 ints.
delete [] array;
int* intPointer = new int(30); // pointer to int with value 30.
delete intPointer;

And yes, a for loop is just a while loop with the added convenience of including the logic inside the for (...) part. There are three statements inside the for, and all three are optional.

The first is run once before the loop begins. It is generally used to initialize a variable.

The second is the condition. Before each time through the loop, that statement is checked. If it is true, the loop is run again. If it is false, then the loop is done.

The third is run at the end of each run through the loop. It is generally used to increment the variable used in the condition.

Your use of the for loop was almost correct, but you have to remember that the condition must be true for the loop to run. That is why a typical for loop looks like:

for(int i = 0; i < MAX; i++)

Initialize i to 0; run while i is less than MAX; increment i by one each time through the loop.