4

I am having a problem with an exercise that asks me to receive two integers and print them. But, the program ends when the user enters with the entry '|'. However, I am testing this and the program enters in an infinite loop. What is the problem?

#include <iostream>
using namespace std;

int main ()
{
  int i1 = 0, i2 = 0;

  cin >> i1;
  cin >> i2;
  while (i1 != int('|') && i2 != int('|'))
  {
     cout << i1 << endl;
     cout << i2 << endl;

     cin >> i1 >> i2; 
  } 

  return 0;
}
JeJo
  • 30,635
  • 6
  • 49
  • 88
  • 2
    You need to read a few good [books](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). – Jesper Juhl Jun 23 '18 at 13:55
  • 3
    Downvoters, instead of downvoting, please, explain our newcomer what's wrong with his question and when/how to ask a question in Stackoverflow – Just Shadow Jun 23 '18 at 14:04
  • Instead of entering the `|` character, try entering the number 124. Your program should exit, instead of looping forever. – AJNeufeld Jun 23 '18 at 14:13
  • Yep, but the question is still interesting... why infinite loop happens in case of '|' – Just Shadow Jun 23 '18 at 14:17
  • 2
    The stream is entering a fail state when an invalid input is received; it won’t recover until that error state is cleared. – AJNeufeld Jun 23 '18 at 14:19
  • Thank you very much, guys for the support and the tips. I promise that I will study and search more. Thanks a lot. – Felippe Trigueiro Jun 23 '18 at 14:23

2 Answers2

3

When you std::cin the non-integer type (charector '|') inside the loop, it fails. Use std::cin.fail() check that.

For example, run the following, and you will get the idea why this happened:

while (i1 != int('|') && i2 != int('|'))
{
    std::cout << i1 << endl;
    std::cout << i2 << endl;

    std::cin >> i1 ;  // std::cin fails here, in the the case of '|'
    if(std::cin.fail()) { std::cout << "Failed"; break;}

    std::cin >> i2;  // std::cin fails here, in the the case of '|'
    if(std::cin.fail()) { std::cout << "Failed"; break;}
}

Above will fix the code. However, you can also write a code for any case of std::cin failure, by checking with std::cin::fail().

while ( std::cin >> i1 && !std::cin.fail()   // check wether i1 failed, if not continue
     && std::cin >> i2 && !std::cin.fail() ) // check wether i2 failed, if not continue
{
   std::cout << i1 << "\n" << i2 << std::endl;
}

Update: As @AJNeufeld pointed out while (i1 != int('|') && i2 != int('|')) will fail to read 124, even the inputs are integers(which is equal to the ASCII code of the vertical pipe character).

A possible solution is to read both values as strings, check for the ‘|‘ character, if not present, convert the strings to ints, or report errors or break the loop. (Credits to @AJNeufeld)

NathanOliver
  • 171,901
  • 28
  • 288
  • 402
JeJo
  • 30,635
  • 6
  • 49
  • 88
  • Thanks for the answer. It helps a lot. – Felippe Trigueiro Jun 23 '18 at 14:24
  • Your first “proper fix” will fail if the user enters `124` for either i1 or i2! Both that and the “alternatively” version will fail (terminate the loop) on any invalid input, such as “x”, instead of only when “|” is given. – AJNeufeld Jun 23 '18 at 15:00
  • OP asked for: *the program ends when the user enters with the entry '|'* i,e. on the press of char `|`, OP need a loop break, not in the case of an integer(`124`). *Alternative* would be a wrong word to chose may be. Sorry for misleading. What I meant is, to break the loop for any case of `std::cin` failure. – JeJo Jun 23 '18 at 15:07
  • From the problem description and the OP’s attempt `while (i1 != int('|') && i2 != int('|')) {…}` it looks like the exit condition is supposed to be ‘|’ character, not just any invalid input. – AJNeufeld Jun 23 '18 at 15:14
  • @AJNeufeld Yes. Now I realise why the confusion is. You are right. Still, the question is whether he wants to store 124 as an int or a loop break condition? If OP needs also a break, condition `i1 != 124` && `i2 != 124` has to be included. Not sure about OPs desion here. BTW thats for this edge case :) – JeJo Jun 23 '18 at 15:19
  • 1
    And for clarity, `124 == int('|')` and `124 == static_cast('|')` are both true because the ASCII code for the vertical pipe character is 124. So the loop will unexpectedly break on the valid Integer input of 124. – AJNeufeld Jun 23 '18 at 15:20
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/173679/discussion-between-jejo-and-ajneufeld). – JeJo Jun 23 '18 at 15:22
  • the cast is useless because the char will be promoted to int anyway – phuclv Jun 23 '18 at 15:54
3

When you use >> to extract a value from an istream, the original stream is returned from the expression. This mean you can replace this code ...

std::cin >> i1;
std::cin >> i2;

with this code:

std::cin >> i1 >> i2;

The result of that expression is again the original istream, and when used in a boolean context, returns false if the stream is in a “fail” state. Thus, we can read the two integers and test if that was successful in one simple construct:

if( std::cin >> i1 >> i2 ) {
    std::cout << i1 << "\n" << i2 << "\n";
} else {

The above stream will enter a fail state, when it is trying to read an integer and encounters a non-integer character, such as the vertical pipe. To test for this, we need to first clear the error state, then see what character was in the stream that caused the integer read to fail. We can do this by peeking at what the next character is.

} else {
    std::cin.clear();    // reset the fail state
    if (std::cin.peek() == '|') {
        std::cout << "Found end of input marker (|)\n";
    } else {
        std::cout << "Invalid input!";
    }

After determining whether it was the vertical pipe or not, it is wise to clear out all characters in the stream up to the end of the input line.

    // skip all characters in the stream up to the end of the current line.
    std::cin.ignore(std::numeric_limits<streamsize>::max(), '\n');
}

Put the above code in a loop, add a break statement when the vertical pipe is found, and you will be looping, reading integer values in pairs, until you type in the |.

while ( !std::cin.eof() ) {
    if( std::cin >> i1 >> i2 ) {
        std::cout << i1 << "\n" << i2 << "\n";
    } else {
        std::cin.clear();    // reset the fail state
        if (std::cin.peek() == '|') {
            std::cout << "Found end of input marker (|)\n";
            break;
        } else {
            std::cout << "Invalid input!";
        }
        // skip all characters in the stream up to the end of the current line.
        std::cin.ignore(std::numeric_limits<streamsize>::max(), '\n');
    }
}
JeJo
  • 30,635
  • 6
  • 49
  • 88
AJNeufeld
  • 8,526
  • 1
  • 25
  • 44