Infinite Loop for Certain Entries

[thomasbeecher]

I have completed this project for my Computer Science I class. I received full credit because the program is functional with no issues at a glance. However, upon testing the program again (after it was graded), I found that under specific circumstances an infinite loop occurs. I couldn't go on with life until I figure out what I've done wrong! I have posted the full instructions below for reference:

An approximate value of pi can be calculated using the series given below:

pi = 4 [ 1 - 1/3 + 1/5 - 1/7 + 1/9 . . . + ((-1)^n)/(2n + 1) ]

Write a C++ program to calculate the approximate value of pi using this series. The program takes an input n that determines the number of terms in the approximation of the value of pi and outputs the approximation. Include a loop that allows the user to repeat this calculation for new values n until the user says she or he wants to end the program.

Savitch, Walter J., and Kenrick Mock. "Chapter 3." Problem Solving with C++. 8th ed. Boston: Pearson Addison Wesley, 2012. 174. Print.

Here is the completed program:

#include <iostream>
#include <cmath>
using namespace std;

int main()
{
    int terms;
    double pi = 1;
    char restart = 'N';

    // Do the entire program as long as the user wants
    do
    {
        // Set pi back to 1 if user wants to restart
        if ((restart == 'Y') || (restart == 'y'))
            pi = 1;

        cout << "Input the number of terms to approximate pi to.  The number of terms must be greater than zero." << endl;
        cout << ">> ";
        cin >> terms;

        // The number of terms must be greater than 0 for approximation purposes
        if (terms > 0)
        {
            // The series approximates to the value terms times
            for (int i = 1; i <= terms; i++)
            {
                pi += 4 * (pow(-1,i))/((2*i)+1);
            }

            // Once loop is complete, add 3 for final pi value
            pi += 3;

            cout << "The approximated value of pi is: " << pi << "." << endl << endl;

            // Allow the user to restart program
            cout << "Enter Y to start a new approximation or any other key to terminate the program." << endl;
            cout << ">> ";
            cin >> restart;
            // A new line is inserted for spacing if the user wants to restart
            cout << endl;

        }
        // Stop the program if the user enteres an invalid value.
        else cout << endl << "You did not enter a valid value.\n" << endl;
    } while ((restart == 'Y') || (restart == 'y'));

    return 0;
}

Steps to produce error:

1. Run program. Enter a number, as requested.
2. Results are shown. Hit Y to restart program.
3. Instead of typing a number, type a letter.
4. Infinite loop occurs.

If the program is started and a letter is entered instead of a number, it does reject it and will end the program. The infinite loop occurs only after the program has been run at least one time. What it should do is always reject invalid entries, even after the program has been run once.

I would like advice on how I can make the program always stop the user if the user enteres incorrect information. It would also be nice to know how I can make the program restart if the user enters incorrect information instead of the program just ending.

I am constantly trying to improve my problem solving and programming skills, so offer advice for anything you see that could be better! Thank you.

[DavidB]

I think you should do more thorough checking of terms between lines 20 and 23. It may be a letter, and have a value greater than 0, which satisfies the if condition, but you should do a better check that it is, indeed, a number before line 23.

What if you put an output statement before line 23, indicating if terms is greater than 0, enter some letters, then run a few tests to see how letters are being treated. For example, add the following code just after you input terms on line 20:

cout << "The input value of terms is: " << terms << "." << endl << endl;
if (terms > 0) {
  cout << "The value of terms has been identified as being greater than 0." << endl;
}

Then run the program a few times entering a non-number. Then you will see how your program is handling non-numbers. I suspect it is not handling them properly, and entering that if block when it should not.

[NathanOliver]

This is a classic case of stream corruption. You asked for a number and got a letter so after that all stream operations will fail. one way to stop this is to use a loop when asking for inputs. here is an example:

int num;
do
{
    cout << "Enter a number: ";
    if (!(cin >> num))
    {
        num = 0;
        cout << "Invalid input. Please try again\n";
        cin.clear();
        cin.ignore(numeric_limits<streamsize>::max(), '\n');
    }
} while (num = 0)

[thomasbeecher]

NathanOliver, could you explain exactly what is going on with the snippet you wrote? I just started learning C++ and I don't really have a clue what that all means. Is there a simpler way to check for valid data?

[NathanOliver]

if (!(cin >> num)) means if cin tries to get a number and fails then enter the if condition.

cin.clear() get rid of the error flags in cin and allows you to use the stream again.

cin.ignore(numeric_limits<streamsize>::max(), '\n'); means throw out all characters in the in buffer un till you either a newline character or you read in the maximum characters that the buffer can hold.

All the code does is see if you got something that cin wasn’t expecting. When you call cin >> num cin knows that you are trying to read in an integer. When you supply a character the read fails since it is expecting a number. After the read fails cin's error state flag gets set to true. While the flag is true you will not be able to read any more information out of the stream. So we call cin.clear() to reset the error flag to false. After we do that we want to make sure that the buffer is empty hence the call to cin.ignore().

As far as an easier way to check for valid input there might be but nothing is coming to mind at the moment. You might want to take all of your data in as a string and process the string but I think that would be more complicated than what i have presented to you.