1

I've got a fairly long program I'm working on and am having difficulty deleting an element from a vector. I've tried to do it with a very simple vector, and am having the same problem. As far as I can see, I've done it the same way everybody has explained in other people's questions. Here is the simple code. 

vector<int> vect;
vect.push_back(3);
vect.push_back(2);
cout << vect[1];  // prints '2'
vect.erase(vect.begin()+1);
cout << vect[1] << endl; // prints '2'

What am I doing wrong?

It seems the above code works, because I checked the size at the end, and it printed '1'. The real code doesn't though:

size = A[i]->B().size();
cout << "size is " << A[i]->B().size() << endl;  // prints 21
A[i]->B().erase(A[i]->B().begin()+size);
cout << "size now " << A[i]->B().size() << endl;  // prints 21

I can't see what I've done differently? A is a vector, which stores other vectors. I want to delete the last element in the B vector.

CocaCola
  • 751
  • 3
  • 10
  • 21
  • 2
    +1 for producing a simple example ;) – Matthieu M. Oct 28 '12 at 14:27
  • You forgot to explain what _different_ behaviour you expected! – Lightness Races in Orbit Oct 28 '12 at 14:28
  • @Sandra Regarding your edit, `A[i]->B().begin()+size` points to *one beyond the last element*. So, that line reads `A[i]->B().erase(A[i]->B().end());` and nothing is erased. To erase the last element, `A[i]->B().pop_back();` – Praetorian Oct 28 '12 at 14:42
  • I've edited my question with my actual code - I'm expecting the last element in 'B' to be deleted. – CocaCola Oct 28 '12 at 14:42
  • @Praetorian I changed it to begin()+size-1 and it is still size 21 on the last line. – CocaCola Oct 28 '12 at 14:44
  • @Sandra [Here's](http://liveworkspace.org/code/708ddae898a6a461f91da52b6adea514) a demo (I'm using C++11 features, but that's not relevant). Anyway, to remove the last element, use the `pop_back` member function. – Praetorian Oct 28 '12 at 14:48

3 Answers3

5

After you erase your element, your vector's size becomes 1 (because it was 2 before the erasing), essentially making your expression vect[1] result in undefined behavior, because there is no element with index 1 any more. All is left is a single element (value = 3, index = 0). If you used vect.at(1) instead of vect[1], it would throw std::out_of_range.

After your edit: Remember, if the size is N, then N is not a valid index for the vector!!! The elements are indexed 0, 1, 2, ... N-1. So indeed, the size is 1, therefore the only valid index is 0

Armen Tsirunyan
  • 130,161
  • 59
  • 324
  • 434
2

What you are doing is undefined behavior. Basically, you are accessing past the currently last element of the vector, and you happen to find garbage that was left here. The problem is made evident using at instead of [] for accessing the element because at has built-in range checking.

// cout << vect[1] << endl;
cout << vect.at(1) << '\n';

If you replace [] by at as above, you will get a std::out_of_range exception signalling that the index you provided is invalid.

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
  • Ha, I was adding the part about at in my answer when you posted yours. +1 – Armen Tsirunyan Oct 28 '12 at 14:31
  • 2
    @ArmenTsirunyan: I just wish that C++ was safer by default and `[]` had range checking while `at` did not. Newbies are irremediably drawn to the lighter weight syntax (so reminiscent of arrays in plenty of other languages) and run afoul of undefined behavior in a heart beat :( – Matthieu M. Oct 28 '12 at 14:32
  • 1
    I believe MSVC does perform range checking for `operator[]` in Debug builds by default. That is a handy feature to have, and you don't pay the penalty for the check in Release builds. – Praetorian Oct 28 '12 at 14:40
  • 1
    @Praetorian: Dirkumware's implementation contains a number of checks that can be activated using [`_HAS_ITERATOR_DEBUGGING`](http://stackoverflow.com/questions/6103314/visual-studio-debug-iterators) or `_SECURE_SCL`. If I remember right, in case of violation you will hit an `assert` and your program will abort. – Matthieu M. Oct 28 '12 at 14:52
1

To delete the last elemnt in a vector, you can use .pop_back(). Also you should note that adding integers to an iterator is a bad idea. Use vector<int>::iterator instead as your variable to add.

The Great Sauron
  • 117
  • 5