Jump to content

  • Log In with Google      Sign In   
  • Create Account

help with finding a string in a vector and removing it.


Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.


  • You cannot reply to this topic
2 replies to this topic

#1 Batistab   Members   -  Reputation: 100

Like
0Likes
Like

Posted 04 May 2011 - 03:05 PM

Hi guys first post and I should start it by saying thanks to everyone that post here, many of your Q&A's have helped me out.

So i'm learning how to program with C++ through "Beginning C++ through game programming, 3rd ed".
Which i want 2 say is a great book and anyone that wants to learn C++ through examples, test and a friendly type of writing style. It's all console programing and there isn't any real GFX game programming to speak of.

So i need help with one of the exercises at the end of intro to STL chapter. What hes asking is:
"Write a program using vectors and iterators that allows a user to maintain
a list of his or her favorite games. The program should allow the
user to list all game titles, add a game title, and remove a game title."

so far this is what I've got:

#include <iostream>
#include <vector>
#include <string>
#include <algorithm>
#include <iterator>


using namespace std;

int main() {

	vector<string> favGame;
	vector<string>::iterator iter;
	iter = favGame.begin();
	vector<string>::iterator othIter;
	string other;
	string done = ("no", "No", "Done", "done");
	other != done;
	string erase;
					
	
				cout << "\tWhat are your favourite games?";			
				cout << "\nList of favourite games? ";
				cout << "\nIf the list is done just say so.\n";
while (other != done)	{ {
				cout << "Game: ";						
}
					cin >> other;				// this is where i insert the new string into the list.
		othIter = favGame.insert(favGame.begin(), other);
}
		

		othIter = favGame.erase(favGame.begin()); 				// this is so that 'done' doesnt appear in the displayed string.
				cout << "\n\n\tUpdated list: \n";
for (iter = favGame.begin();iter < favGame.end();++iter) {
				cout <<	*iter << endl;
}

		
				cout << "\n\n\tWould you like to erase a game?\n";
while (erase != done) { { 						// this is where the problem is, the program runs fine if i take out this chunk.
				cout << "Erase: ";
}

				cin >> erase;  			//really i think im going about this whole chunk of code wrong. i think an if statement...
			othIter = find(favGame.begin(), favGame.end(), erase);		 // that goes like if erase == ( string in favGame), delete string...
				favGame.erase(othIter);								// else game not found.
}

		
				cout << "\n\n\tUpdated list: \n";
for (iter = favGame.begin();iter < favGame.end();++iter) {
				cout <<	*iter << endl;
}}

My Problem is i need a way to enter a string into the console and then find it in favGame and erase it.
Also how is my layout of the code, any pointers you want 2 give.

#2 rip-off   Moderators   -  Reputation: 10547

Like
2Likes
Like

Posted 04 May 2011 - 04:20 PM

You've got a few problems in your code. I'll address them in no particular order.

Your string "done" is actually (due to the unfortunate "comma operator") only equal to "done". If you want an array, you'll need to change it to:
string done [] = {"no", "No", "Done", "done"};
You can then use std::find() on this array. However, for the moment I would recommend using a function:
bool done(std::string input)
{
    return input == "no" || input == "No" || /* ... */;
}
This is a little simpler than using std::find on an array (which involves pointer arithmetic).

In fact your entire program would be much more readable if it was split into a number of functions:
void readGames(vector<string> &games);
void eraseGame(vector<string> &games);
void displayGames(const vector<string> &games);

int main()
{
    vector<string> games;

    readGames(games);
    displayGames(games);
    eraseGame(games);
    displayGames(games);
}

The line "other != done" doesn't do anything. It just checks if other and done aren't the same, and throws away the result of that test. If you want to start such that other is not equal to done, you don't have to do anything special, by default std::strings are empty and an empty string is not equal to any of the elements of your array (they're all non-empty).

It is normal to use "push_back" to append items to a list, unless you really want to add your items in the manner you were doing.

You should scope variables as tight as possible. This means not declaring them until you are going to use them, and also initialising them during declaration where possible. This is especially important with types like iterators which can be invalidated easily - any function that structurally modifies a vector could invalidate its iterators.


I'll show you a neat trick for handling input in C++. You want to output a prompt, then read a variable. You can do this:
string input;
while(cout << "Prompt: " && cin >> input)
{
    // Use input!
}
Remember that the stream operations return the current state of the stream. We can convert this into a boolean to see if the stream is "good", this means that you can read/write to it. The above loop is not only a cheeky way to get a prompt before reading a variable, it is also better because if for some reason the input or output streams fail we will notice and terminate the loop.

We can combine this with the "done" function from before to get:
string input;
while(cout << "Prompt: "  && cin >> input && !done(input))
{
   // ...
}
This avoids the problem you had where you had to erase the last game the user enters because it would be one of the stop words.

The canonical loop for iterating over a constainer is the following:
for(container<Type>::iterator i = c.begin() ; i != c.end() ; ++i)
{
    // use *i
}
Note that we use != rather than < for the condition. This works well for all containers, whereas < only works with random access ones like vector.

Your actual problem is that you aren't checking if the item was found before trying to erase it. The return value of std::find() is either a valid element in the container (i.e. one you could erase()), or the end() iterator, which you should not dereference or erase.

Here is a cleaned up version of your code illustrating some of these ideas.
#include <iostream>
#include <vector>
#include <string>
#include <algorithm>
#include <iterator>


using namespace std;

bool done(string input)
{
	return input == "no" || input == "No" || input == "done" || input == "Done";
}

void readGames(vector<string> &games)
{
	cout << "\tWhat are your favourite games?";                     
	cout << "\nList of favourite games? ";
	cout << "\nIf the list is done just say so.\n";

	string input;
	while (cout << "Game: " && cin >> input && !done(input)) 
	{ 
		games.push_back(input);
	}
}

void eraseGames(vector<string> &games)
{
	cout << "\n\n\tWould you like to erase a game?\n";
	string input;
	while (cout << "Erase: " && cin >> input && !done(input)) 
	{ 
		vector<string>::iterator i = find(games.begin(), games.end(), input);
		if(i == games.end())
		{
			cout << "Couldn't find: " << input << '\n';
		}
		else
		{
			games.erase(i);
		}
	}
}

void displayGames(const vector<string> &games)
{
	cout << "Games list: \n";
	for (vector<string>::const_iterator i = games.begin() ; i != games.end() ; ++i)
	{
		cout << *i << endl;
	}
}

int main()
{
	vector<string> games;
	
	readGames(games);
	displayGames(games);
	eraseGames(games);
	displayGames(games);
}
It would be easy to wrap the body of the program in another loop with a basic menu system, if you'd like an additional task to try out.

#3 Batistab   Members   -  Reputation: 100

Like
0Likes
Like

Posted 04 May 2011 - 05:22 PM

Great answer man, this was really helpful and when i get home i'll try these changes and give you the heads up on the results.
thank you.




Old topic!

Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.




PARTNERS