Member Avatar for 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.

  • 3 Contributors
  • 4 Replies
  • 778 Views
  • 21 Hours Discussion Span
  • Latest Post Latest Post by NathanOliver

Dani AI

Generated

Diagnosis and short explanation (ties to and )
The “infinite” loop is actually stream failure. In ’s run, typing a letter when cin >> terms expects an integer sets cin into a fail state and leaves terms unchanged (it keeps the previous positive value). Because the code only tests if (terms > 0) it proceeds with the calculation. Later cin >> restart also fails (stream still in error), so restart remains 'Y' and the loop condition keeps being true while all extractions immediately fail. This is the stream-corruption behavior named; ’s advice to validate terms is correct but won’t help unless the code also detects extraction failure.

Minimal fixes (fast changes)
After every cin >> of a numeric type check whether the extraction succeeded. If it failed, call cin.clear() and then discard the rest of the line (e.g. cin.ignore(...)) before re-prompting or aborting. That prevents the leftover bad characters from being re-used and stops restart from being stuck.

Robust alternative (recommended)
Read a whole input line and parse it. This cleanly detects non-numeric characters and leaves cin usable:

#include <sstream>
#include <string>

// read a positive integer from a single line
int terms;
std::string line;
while (true) {
    std::cout << "Input number of terms >> ";
    if (!std::getline(std::cin, line)) break; // handle EOF
    std::stringstream ss(line);
    if (ss >> terms && ss.eof() && terms > 0) break;
    std::cout << "Invalid input; enter a positive integer.\n";
}

Calculation robustness
Reset and compute pi from scratch each run (don’t accumulate from prior runs) and prefer a simple sign flip instead of pow(-1, i):

double sum = 0.0;
int sign = 1;
for (int k = 0; k < terms; ++k) {
    sum += sign / double(2*k + 1);
    sign = -sign;
}
double pi = 4.0 * sum;

Notes: handle EOF (Ctrl-D/Ctrl-Z) if needed, and validate the restart answer the same way (read a line and test the first non-space character). These changes will prevent the observed infinite-loop symptom and make input behavior deterministic.

Recommended Answers

All 4 Replies

Member Avatar for 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.

Edited by DavidB
Member Avatar for 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)
Member Avatar for 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?

Member Avatar for 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.

Edited by NathanOliver
Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.