views:

363

answers:

5

I have a loop where I ask the user to enter a name. I need to stop when the user presses the ENTER key..... or when 20 names have been entered. However my method doesn't stop when the user presses the ENTER key

//loop until ENTER key is entered or 20 elements have been added
bool stop = false;
int ind = 0;
while( !stop || ind >= 20 ){

    cout << "Enter name #" << (ind+1) << ":";
    string temp;
    getline(cin, temp);
    int enterKey = atoi(temp.c_str());        

    if(enterKey == '\n'){
        stop = true;            
    }
    else{
        names[ind] = temp;
    }

    ind++;


}
+2  A: 

try:

while( !stop && ind < 20 )

or:

using namespace std;
vector <string> names; // edited.
for (int ind = 0; ind < 20; ++ind)
{
    cout << "Enter name #" << (ind+1) << ":"; 
    string temp;
    getline(cin, temp); 
    if (temp.empty())
        break;
    names.push_back(temp);
}
Charles Beattie
Actually it's a bit of a mess. There are a few mistakes...
Charles Beattie
Yes, I completely agree, a `for` loop is the way to go. Your code has a bug though. The resulting vector will have 20 empty strings followed by some number of responses.
Geoff
You left out the atoi step. You should have used a vector<int> not vector<string>
jmucchiello
@James: He should be checking both the length of input (to detect the empty line) and he should be checking the stream state because that's just good coding.
jmucchiello
@jmucchiell, OP did not insert the int into the container, the int was only being used to detect a new line.
Geoff
Thanks @Geoff edited to be correct. Changed mind half way through typing it out was going to use names[ind] so user69514 wouldn't get too confused but went with best practice instead...
Charles Beattie
+2  A: 

getline will eat your delimiter, which will be '\n', so you probably want to be checking for an empty string. Do it before the call to atoi.

LukeN
+1  A: 

Try stop = temp.empty() instead. getline should not contain any new-line characters. An empty line should result in an empty string.

Also, Charles is correct, your while condition is incorrect, use while( !stop && ind < 20). The way you have it written the user needs to enter 20 values, and an empty line. Charles' change says to break when either condition is met (not both).

For the sake of completeness, here's the proposed new code:

bool stop = false;
int ind = 0;
while( !stop && ind < 20 ){

    cout << "Enter name #" << (ind+1) << ":";
    string temp;
    getline(cin, temp);
    if(temp.empty()) {
        stop = true;
    } else {
        names[ind] = temp;
    }

    ind++;    
}

Personally, I would write the code as follows:

vector<string> names;
for(int ind = 0; ind < 20; ind++) {
  cout << "Enter name #" << (ind + 1) << " (blank to stop): ";
  string name;
  getline(cin, name);
  if(name.empty() || cin.eof()) {
     break;
  }
  names.push_back(name);
}

cout << "Read " << names.length() << " names before empty line detected." << endl;
Geoff
Why not just break when temp.empty()? Then you don't need the else structure
jmucchiello
ah. yeah. should be <. I copied that and missed the error there.
Geoff
You also forgot to flip the ind >= 20 condition.
jmucchiello
I agree, it should be a for loop with a break. If we re-write everything the problems OP originally had may not be clear.
Geoff
I added my personal preference, which I think is in-line with yours @jmucchiello
Geoff
+2  A: 

You convert the read string to an integer with atoi:

int enterKey = atoi(temp.c_str());        

If temp is a string like "1234" this will set enterKey to 1234. Then you compare enterKey to the ASCII value of \n. This is most probably not doing anything useful.

Also std::getline just read the characters up to, but not including, the next '\n'. If a user just presses enter without typing any other characters, std::getline will return an empty string. If a string is empty can be easily tested with its empty() method:

getline(cin, temp);
if (temp.empty()) {
  stop = true;
}
sth
youre right. this worked
A: 

You want to use cin.get(); cin >> temp; I do believe.

DeadMG
`cin >> temp` will only give one token. It's possible OP wants statements that contain spaces.
Geoff