little trouble with simple c++ program

Started by
6 comments, last by bmanruler 16 years, 2 months ago
Just working on some easy exercises in a book i have, as i'm trying to learn c++. I have the following code, and where it asks the user: cout << "Would you like to add another? y/n" << endl; , when y or n is entered, it skips the question in the while loop.(cout << "Please type a name of a game you like." << endl;)

#include <cstdlib>
#include <iostream>
#include <vector>
#include <string>

using namespace std;

int main(int argc, char *argv[])
{
    string game;
    bool addGames;
    char ans; 
    vector<string> games;
    vector<string>::iterator gameIter;
        
    cout << "Please type a name of a game you like." << endl;   
    getline(cin, game);
    games.push_back(game);
    cout << "Would you like to add another? y/n" << endl;
    cin.get(ans);
    if(ans == 'y')
    {
        addGames = true;
    }
    while(addGames)
    { 
        string game;
        cout << "Please type a name of a game you like." << endl;   
        getline(cin, game);
        games.push_back(game);
        cout << "Would you like to add another? y/n"; 
        cin >> ans;
        if (ans == 'n')
        {
            addGames = false;
        }
    }    
    
    cout << "Would you like to remove a game? y/n." << endl;
    cin >> ans;
    
    if(ans == 'y')
    {
        string eraseGame;
        cout << "Please type the name of the game." << endl;
        getline(cin, eraseGame);
        for(gameIter = games.begin(); gameIter != games.end(); ++gameIter)
        {
            if(eraseGame == *gameIter)
            {
                games.erase(gameIter);
            }
        }
    }
    
    if(ans == 'n')
    {
        for(gameIter = games.begin(); gameIter != games.end(); ++gameIter)
        {
            cout << *gameIter << endl;
        }
    }
    
    system("PAUSE");
    return 0;
}


[Edited by - agm_ultimatex on February 12, 2008 1:58:33 PM]
Advertisement
/bump, ideas anybody?
Use [source] tags when posting code to preserve formatting (you can edit your post using the edit button in the upper right).
Idea #1: Post in For Beginners. It's at the top of the forum list for a reason: to get the attention of people who realize their questions pertain to "easy exercises".

Idea #2: Use do-while loops and simplify your logic.

Idea #3: Declare variables near first use, ideally only in the scope where they are used. In particular, declare for-loop counter variables directly in the for-initializer.

Idea #4: Don't #include things you're not going to use (<cstdlib> here). Learn what each header is for.

Idea #5: Think carefully about what will happen if you try to .erase() something inside a container that you are currently iterating over. In particular, think about what will happen if you .erase() the last element, when the loop ends and the iterator is incremented.

The recommended way to remove elements of a container with a given value, is to use the standard library algorithm std::remove(), from <algorithm> You will need to .erase() some "junk" from the end of the container afterwards (the algorithm doesn't know about vectors, it only knows about iterators; therefore, it can't change the number of elements in the container, it can only change the iterated-over elements). That looks like:

games.erase(std::remove(games.begin(), games.end(), eraseGame), games.end());
#1 I apologize, I should have posted this there. I initially put this in the .net one, but it was moved to here.

#3 I know I'm out of practice with HLL, as ive been doing lots of asp lately, so thanks for the help.

#4 Bloodshed Dev-C++ must of put that one in there for the project default, guess I forgot to delete.

#5 I was pretty certain that it would cause problems. I had trouble with a blackjack game assignment on removing players that are out of money and such. But I want to learn that effectively, hence why I'm doing this program. There's little things such as vectors and pointers that I'm not as familiar with as I've done a lot of java.
Your problem is in how you are dealing with the input.

First of all, change cin.get(ans) to cin >> ans. This makes it easier if you change from a char to a different type.

Before I deal with the problem your having first I'll ask that you note that you have some code duplication. There are two copies of the following code:
cout << "Please type a name of a game you like." << endl;getline(cin, game);games.push_back(game);cout << "Would you like to add another? y/n";cin >> ans;


Whenever you see code duplication do your best to remove it. What if you change the prompt in one? You'll have to remember to change the other.

To remove the duplication, we can change the loop from a while loop to the more esoteric do...while loop:
// no need to skimp on variable nameschar answer;do{    string game;    cout << "Please type a name of a game you like." << endl;    getline(cin, game);    games.push_back(game);    cout << "Would you like to add another? y/n";    cin >> answer;}while( answer == 'y' );    

Note that because of this, I have dropped the addGames boolean value, seeing as it is no longer required.


As for what happens in the loop, consider the input you are getting. If you type "y" and hit return, std::cin will contain "y\n" in it's buffer. When you read a single character from that, you will get the 'y'. The newline remains. What happens is that std::getline then reads everything up to the next newline and removes the newline. So, because you have the newline in the buffer, nothing appears to happen (however, an empty string gets pushed into the vector).

To change this, we'll change the way we interpret the input. My favourite technique is to interpret it in a line by line fashion. Its not particularly difficult:
// I've changed the variables type to a stringstring answer;do{    string game;    cout << "Please type a name of a game you like." << endl;    getline(cin, game);    games.push_back(game);    cout << "Would you like to add another? y/n";    // we read the users response as a line into a string    getline(cin,answer);}while( answer == "y" ); // y is now a string literal, not a character    


Finally, look at this code:
if(ans == 'y'){   // remove a game}if(ans == 'n'){   // print everything}

Generally one would write an if else statement to handle this:
if(ans == 'y'){   // remove a game}else{   // print everything}


If you wanted to check that the user entered either a 'y' or a 'n' and do something if they entered neither:
if(ans == 'y'){   // remove a game}else if( ans == 'n' ){   // print everything}else{   // do something else}
Thanks for your help, I know have it working. I had to change the ans to a string and used the getline function, as the problem it proved to a problem as a char. Heres my finished code:
[source code="cpp"]#include <iostream>#include <vector>#include <string>using namespace std;int main(){    vector<string> games;                string answer;    do    {        string game;        cout << "Please type a name of a game you like." << endl;        getline(cin, game);        games.push_back(game);        cout << "Would you like to add another? y/n" << endl;        getline(cin, answer);    }    while( answer == "y" );        string ans;    cout << "Would you like to remove a game? y/n." << endl;    getline(cin, ans);        if(ans == "y")    {        string eraseGame;        cout << "Please type the name of the game." << endl;        getline(cin, eraseGame);        vector<string>::iterator gameIter;        for(gameIter = games.begin(); gameIter != games.end(); ++gameIter)        {            if(eraseGame == *gameIter)            {                games.erase(gameIter);            }        }        vector<string>::iterator gameIter2;        for(gameIter2 = games.begin(); gameIter2 != games.end(); ++gameIter2)        {            cout << *gameIter2 << endl;        }    }        else if(ans == "n")    {        vector<string>::iterator gameIter;        for(gameIter = games.begin(); gameIter != games.end(); ++gameIter)        {            cout << *gameIter << endl;        }    }    else    {        cout << "Please type y or n if you wish to remove a game or not." << endl;        getline(cin, ans);    }        system("PAUSE");    return 0;}
using namespace std; // Don't do this

It is preferable to not include the entire standard library. In this program it isn't a big deal, but in general one would write out this:
using std::vector;using std::string;using std::cout;using std::cin;// etc

This topic is closed to new replies.

Advertisement