Sign in to follow this  
Dko

Vectors and Iterators

Recommended Posts

Dko    146
Well I was doing one of the excersizes in a book of mine. I was saposta have a simple program that lets a person add a game title to a list, deleate one on the list and be able to make the whole list print up. Well its all working except for the deleating a memmber of the list. What I have crashes the program.

#include <iostream>
#include <string>
#include <vector>
#include <algorithm>
#include <ctime>
#include <cctype>

using namespace std;

#pragma warning (disable:4786)

void addTitle();
void removeTitle();
void listAll();
vector<string> gameList;


int main()
{
	int choice;	

	cout << "Hello and wellcome." << endl;

	do
	{
		
		cout << "- 1 to add a game title to your list." << endl;
		cout << "- 2 to remove a title." << endl;
		cout << "- 3 to list all titles." << endl;
		cout << "- 0 to exit." << endl;

		
		cin >> choice;

		switch (choice)
		{
		case 1:
			addTitle();
			break;

		case 2:
			removeTitle();
			break;

		case 3:
			listAll();
			break;
		}
	} while (choice != 0);
                    
	cout << "\nGood bye!\n";
	return 0;
}


void addTitle()
{
	cout << "Please enter Game title to add." << endl;
	string title;
	cin >> title;

	gameList.push_back(title);
}

void removeTitle()
{
	cout << "Please enter Game title to remove." << endl;
	string title;
	cin >> title;

	vector<string>::iterator iter;

	// This is saposta loop thru the vector using a Iterator untill it finds the tittle.
	// Then erase it.  But it crashes the program
	for (iter = gameList.begin(); iter != gameList.end(); ++iter)
	{
		if(*iter == title)
			gameList.erase(iter);
	}
}

void listAll()
{
	vector<string>::iterator iter;
	for (iter = gameList.begin(); iter != gameList.end(); ++iter)
	{
		cout << *iter << endl;
		
	}
	cout << endl;
}

Could someone maybe help me figure out how to get it to work?

Share this post


Link to post
Share on other sites
Guest Anonymous Poster   
Guest Anonymous Poster
It looks to me like when you delete, you are deleting the iterator - therefore the loop goes out of scope.

I think you want to do something like this:


for (iter = gameList.begin(); iter != gameList.end(); )
{
if(*iter == title)
{
// trick to ensure loop incremented and don't delete current iterator
vector<string>::iterator temp = iter;
++iter;
gameList.erase(temp);
}
else ++iter;
// Needs the else clause to ensure loop increments
}
[\source]

I could be wrong though!

Share this post


Link to post
Share on other sites
the problem is your trying to loop through the vector while removing an item from it at the same time... this is asking for trouble. basically your removing the item and then accessing the invalid iterator, which is why its crashing. (i think, please someone correct me because i could be wrong). anyway, theres a trick to do this, it looks like this:


for (iter = gameList.begin(); iter != gameList.end();)
{
if(*iter == title)
gameList.erase(iter);
else ++iter;
}

Share this post


Link to post
Share on other sites
Guest Anonymous Poster   
Guest Anonymous Poster
Or possibly:

gameList.erase(remove(gameList.begin(), gameList.end(), title), gameList.end());

Share this post


Link to post
Share on other sites
Dko    146
Kewl. The moving of the ++iter worked perfectly. Thanks everyone for your help. ^^ I would never of guessed that.

Share this post


Link to post
Share on other sites
Fruny    1658
It still is incorrect. Graveyard filla, first AP, the correct way to write the loop is:


for (iter = gameList.begin(); iter != gameList.end();)
{
if(*iter == title)
iter = gameList.erase(iter);
else ++iter;
}



When you do erase, you need to grab the iterator returned by the function. It will be a valid iterator to the next element.

The suggestion given by the second AP is, of course, the preferred one.

Share this post


Link to post
Share on other sites
whoops, i know that, i know that... [smile] also, for the hell of it, since he only wants to delete one thing, couldnt he just do this:


bool found = false;
for (iter = gameList.begin(); iter != gameList.end(); ++iter)
{
if(*iter == title)
{
found = true;
break;
}
}

if(found)
gameList.erase(iter);





hmm, well, it did look nicer untill i put in the error avoiding (the bool found variable).

Share this post


Link to post
Share on other sites
could someone please explain to me whats going on with this:

gameList.erase(remove(gameList.begin(), gameList.end(), title), gameList.end());

im sitting here staring at the STL docs, but i cant grasp whats going on here. ok, so .erase() will erase all elements in the range of the first parameter to the last parameter. and remove will remove all elements from begin to end, which have the value "title" in this case.it also returns an iterator to the "new last", meaning the new last iterator in the vector after remove removes what it wants. ok.. so....

remove() will go through and remove "title", now what though? it returns an iterator, to say,the last item, and then erase will erase nothing? so why not just use remove() in the first place? hmm, im confused.

thanks for any help.

Share this post


Link to post
Share on other sites
snk_kid    1312
Quote:
Original post by graveyard filla
could someone please explain to me whats going on with this:

gameList.erase(remove(gameList.begin(), gameList.end(), title), gameList.end());

im sitting here staring at the STL docs, but i cant grasp whats going on here. ok, so .erase() will erase all elements in the range of the first parameter to the last parameter. and remove will remove all elements from begin to end, which have the value "title" in this case.it also returns an iterator to the "new last", meaning the new last iterator in the vector after remove removes what it wants. ok.. so....

remove() will go through and remove "title", now what though? it returns an iterator, to say,the last item, and then erase will erase nothing? so why not just use remove() in the first place? hmm, im confused.

thanks for any help.


remove moves all elements equal to the key to the end and returns a iterator to the start of the elements that have been moved to the end so say for example you have this sequence:


1, 3, 5, 3, 4


when you call remove with the key 3 it will do somthing like this:


1, 5, 4, 3, 3


returns an iterator to the first 3 (the value may not be 3 any more thou) then you call erase from that iterator to the end iterator and there all gone so you end up with:


1, 5, 4

Share this post


Link to post
Share on other sites
Fruny    1658
Quote:
Original post by snk_kid

1, 3, 5, 3, 4


when you call remove with the key 3 it will do somthing like this:


1, 5, 4, 3, 3


Not quite, it'll look like this:

1, 5, 4, 3, 4


What happens is that when an element gets removed, the 'read' iterator gets advanced, but the 'write' iterator doesn't. Which means that elements located later in the array start overwriting earlier elements. However the process stops when the read head reaches the end, so if you do have removed, say 2 elements like in your example, the last two elements will be left unmodified. At this point, the algorithm returns the 'write' iterator, which happens to point to the first such leftover element, which you can then take out with erase.

I think the following code would be a correct implementation of remove.
template<class Iter, class Value>
Iter remove(Iter first, Iter last, Value value)
{
Iter write = first;
while(first != last)
{
if(*first != value)
{
if(write != first) *write = *first;
++write;
}
++first;
}
return write;
}

Share this post


Link to post
Share on other sites
Zahlman    1682
...

Given that (i.e. that it's not simply reordering elements), is there any valid way to make use of the elements at the end after a remove()? o_O

Share this post


Link to post
Share on other sites
Fruny    1658
They're leftovers... You really have no reason to keep them around. You could overwrite them with some new data, but you might as well just erase them and *then* add new data when necessary.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this