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.