Issue with a for loop (C++, Visual Studio

Question

I have only really been coding for a few days, though I've reading the textbook for my Intro to C++ class for two weeks. I'm having an issue with an assignment, and I feel like I'm missing something super simple, but I can't understand what I've done wrong.

The exercise calls for you to 'write a program that reads numbers from cin, and then sums them, stopping when 0 has been entered.'

The professor told us we could write it with a for loop, a while loop, or a do-while loop. I am trying to write it with a for loop.

The program compiles successfully, it allows me to enter multiple values, and it sums it correctly. It also stops on no successfully. The only thing that's wrong with it is when I enter 0, it does not stop the program. I have tried using different commands inside the for loop, such as goto, and trying to direct it to go to break; when the value entered is zero, but my knowledge is shoddy, to say the least. I've read the textbook but I don't have enough experience yet, and I don't remember everything, and I can't figure out what I'm doing wrong.

This is what the program looks like:

#include <iostream>

using std::cin;
using std::cout;
using std::endl;

int main()

{
    char indicator{ 'n' };
    double value{};
    double sum{};


    for (;;)
    {

        cout << endl
            << "Enter a value here: ";
            cin >> value;
        sum += value;
        cout << endl
            << "Do you want to enter another value (enter y or n)? ";
            cin >> indicator;
            if (('n' == indicator) || ('N' == indicator))
                break;


    }

    cout << endl
        << "The sum of the values you entered is " << sum << "."
        << endl;
    return 0;

}

Please point out my stupid mistake, I'd be grateful to learn. Thank you!

Answer 1

There is no sense to ask the user each time whether he wants to continue.

So I would write the loop the following way

cout << "Enter a sequence of real numbers (0 - exit): "; 
for (;;)
{
    if ( !( cin >> value ) || ( value == 0.0 ) ) break;
    sum += value;
}

Also as the variable value is used only in the body of the loop it should be declared there. So the loop can look like

cout << "Enter a sequence of real numbers (0 - exit): "; 
for (;;)
{
    double value;

    if ( !( cin >> value ) || ( value == 0.0 ) ) break;

    sum += value;
}

An alternative for this for loop is while loop of the following form

cout << "Enter a sequence of real numbers (0 - exit): "; 
while ( true )
{
    double value;

    if ( !( cin >> value ) || ( value == 0.0 ) ) break;

    sum += value;
}

Answer 2

Just compare value to zero after it is inputted:

for (;;)
{

    cout << endl
        << "Enter a value here; enter 0 to stop: ";
        cin >> value;
   if(value==0.0) break;
   sum += value;
}

Answer 3

I would like to make a few suggestions to you which you may find helpful.

To begin, any of the loops you mentioned (for, while, do while) may be used in solving this problem. But I believe that the do while lends itself best to this particular problem.

The use of a for is especially bad for this exercise as for loops are typically employed while performing iterations over a set of values or some sort of finite counting. Being asked to perform an action an indefinite number of times is better suited for a while or do while.

Just look at your for declaration:

for(;;)
{
    // ...
}

What benefit does using for have in this situation if you make use of none of it's functionality.

Second, the use of double is for your value and sum is not recommended either. This is because of something called floating-point precision. You can not reliably compare a double value using ==. See: What is the most effective way for float and double comparison?

Instead, unless specified by the problem statement, I would opt to use an integer value type. Either int or unsigned int.

Third, you are not correctly initializing your variables. Instead, it should be done as:

int value = 0;
int sum = 0;

Improper variable initialization can lead to countless bugs and is a very common source of problems.

Finally, I would recommend against the use of using namespace. This is used commonly by beginner developers who then are taught better practices and then have to unlearn the behavior. See: Why is "using namespace std" considered bad practice?

Taking this advice into consideration would lead to a solution such as:

#include <iostream>

int main(int argc, char** argv)
{
    int sum   = 0;
    int value = 0;

    do
    {
        sum += value;
        std::cout << "Please enter a value: ";
        std::cin >> value;
    } while(value != 0);

    std::cout << "The sum of all values entered is: " << sum;

    return 0;
}

Answer 4

You can just change

cout << endl
        << "Do you want to enter another value (enter y or n)? ";
cin >> indicator;
if (('n' == indicator) || ('N' == indicator))
    break;

with

cout << endl
        << "Do you want to enter another value (enter 0 for exit or not zero to continue)? ";
int indicator = -1;
cin >> indicator;
if (indicator == 0)
     break;

If you do not want to ask user to continue enter new value in every time @Govind Parmar and @Vlad from Moscow write cleaner code than you write.