1

I am learning C++, and I am doing some exercises in the book I am using. One of them asks to write a program that asks a user how many numbers they want to add up. Then prompt for the numbers the user wants to add or to enter '|' once finished. The numbers are then pushed into a vector. Part of the program asks to check if the size of the vector is equal to the original number of input items and that is where I keep getting an error.

cout << "Please enter the numbers and | once you are done: ";

while(true)
{

    for(int num; cin >> num; )
    {
        if(num == '|')
        {
            break;
        }

        ints.push_back(num);
    }

    if(ints.size() != n)
    {
        cout << "There are more or less numbers in the vector than originally specified\n"
            << "Vector will be cleared; please re-enter the values: ";
        ints.clear();
        continue;
    }
    else
    {
        break;
    }
}

The problem is that if the number of input is off, the message goes into an infinite loop and I am not sure how to fix it.

EDIT: n is the variable that holds in the number of values user wanted to enter.

Thanks!

Assafs
  • 3,257
  • 4
  • 26
  • 39
Jesus
  • 63
  • 3
  • 1
    `num` is an integer and `cin >> num` won't extract `|` symbol. Comparison `num == '|'` may not work as expected because `num` could have the numeric value of `|` ascii symbol even when user did not input any `|` symbol. – user7860670 Sep 04 '17 at 18:23
  • @VTT are you sure? Because it works with correct input. Once it reads in the | it breaks out and actually goes on to add up all the numbers. I thought the ' ' meant character value which can be read as an int. Also, exercise asks to use | to break so this wasn't up to me. – Jesus Sep 04 '17 at 18:25
  • 3
    Just try entering `124` (that is numeric value of `|`), it will break the loop. – user7860670 Sep 04 '17 at 18:27
  • One thing wrong is that your number-collecting loop runs one iteration _before_ reading the first number. Just make it a `while(true)` as well, and read from `cin` inside it. – Rob Starling Sep 04 '17 at 18:41
  • Also, i think the code would be clearer to SO-readers if the first line were: `cout << "Please enter " << n << " numbers, followed by | (vertical bar): " << flush;` – Rob Starling Sep 04 '17 at 18:43
  • Is there are requirement or assignment saying to use `'|'` as the terminating character? There was a similar question last week. – Thomas Matthews Sep 04 '17 at 18:48
  • 1
    Your problem is `cin` that enters an error state and stops reading further input. You need to clear this state, see `std::istream::ignore` and `std::istream::clear`. – n. m. could be an AI Sep 04 '17 at 18:51
  • @Jesus The best tool to find such bugs is to use your debugger and step through your code line by line. – user0042 Sep 04 '17 at 18:53
  • Can you explain the reason of using a 'while(true)' loop? I have been taught that it is generally really bad practice to do something like this. – Sailanarmo Sep 04 '17 at 19:09
  • @Sailanarmo I guess it is because `while(true)` contains a condition that is a constant. A better approach for an endless loop would be Cthulhu loop or use of real exit condition that is set inside of the loop. – user7860670 Sep 04 '17 at 19:12
  • @VTT OP know's his conditions however, if the size of the vector is not equal to the size of the number of integers in his vector, then clear the vector and prompt the user to repeat. – Sailanarmo Sep 04 '17 at 19:14
  • @Sailanarmo I guess it could be converted to `while(ints.size() != n)` but the thing is this condition also needs to be checked in the middle of the loop anyway. – user7860670 Sep 04 '17 at 19:21
  • Possible duplicate of [Infinite loop with cin when typing string while a number is expected](https://stackoverflow.com/questions/5864540/infinite-loop-with-cin-when-typing-string-while-a-number-is-expected) – Ripi2 Sep 04 '17 at 19:42

2 Answers2

2

num is an integer and cin >> num won't extract | symbol. Comparison num == '|' may not work as expected because num could have the numeric value of | ascii symbol even when user did not input any | symbol. You should properly handle end marker reading:

// loop will break when user enters `|` because it is not an integer
// setting failbit of cin
for(int num; cin >> num;)
{
    ints.push_back(num);
}
cin.clear(); // reset failbit making cin able to read again
// check the end marker entered by user
{
    string end_marker;
    cin >> end_marker;
    if("|" != end_marker)
    {
        // use of endl will flush the stream ensuring that
        // printed text won't stuck in the buffer
        cout << "Please use | as end marker" << endl;
        continue;
    }
}
user7860670
  • 35,849
  • 4
  • 58
  • 84
0

Here is how I implemented it. I am worried about the logic in your while loop. I had been taught to avoid while(true) whenever possible. You know the logic behind how your code should work. With more practice you'll start to recognize the conditions you need to use. I am sure there are better ways to do it. But this is the way I tried it.

But to answer your question, the main reason it is failing is because integers cannot compare themselves with characters. 

if(num == '|')

That does not work since num is an integer and not a character.

Normally I would implement this in a class and since global variables are not highly looked upon I created my own namespace. You'll have to finish the rest of the logic yourself however:

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

namespace global
{
    std::vector<std::string> strings;
    std::vector<int> ints;
    std::string a = " ";
    int num = 0;
}   

void doWork()
{   

    std::cout << "Please enter the number of integers you would like to add up: ";
    std::cin >> global::num;

    std::cout << "Please enter the numbers and | once you are done: ";
    while (global::a != "|")
    {
        std::cin >> global::a;
        global::strings.push_back(global::a);
    }

    global::strings.pop_back();

    for(auto &e : global::strings)
    {
         global::ints.push_back(std::stoi(e));
    }
}   


int main()
{

    doWork();

    if(global::ints.size() != global::num)
    {
        std::cout << "Size of vector does not match the size specified. Clearing vector" << std::endl;
        global::ints.clear(); 
        global::strings.clear();
        global::num = 0;
        global::a = " ";
        doWork();
    }   

}

I made a vector of char's and converted those into integers so that way you could add them up. The while loop should be checking for | rather than always running true. It then will check the size of the vector in the end, clear it if it does not match, and ask you to do it again. This is the best way that I could think of doing it.

EDIT: as VTT pointed out, char can only do one character at a time. I have converted it into a string in order to handle the conversion.

EDIT 2: reset the values of global::num and global::a to their default at the end of the failure in order to prevent crashing.

Sailanarmo
  • 1,139
  • 15
  • 41
  • This code is still wrong because it will read individual chars instead of integers. That is if user enters 123 `global::ints` will contain 3 numbers `1`, `2` and `3`. – user7860670 Sep 05 '17 at 08:04
  • @VTT I have now edited it to use `std::string` instead of `char`. There is an issue that I am trying to figure out, the program keeps crashing when it clears the vector and asks for user input again. – Sailanarmo Sep 05 '17 at 08:41
  • @VTT the program should run flawlessly now. I still think this is the best way to handle it. It's definitely not pretty, and I would rather make a class for this in order to handle the global variables a lot better, but this is a lot safer than using a `while(true)` loop to handle the logic. Thank you for pointing out my errors. – Sailanarmo Sep 05 '17 at 08:47