simple c++ for loop unexpected output

Question

I have been writing a program which asks you to input two integers, which then lists all integers from the smaller of the two integers entered to the larger of the two integers entered (inclusive). I want the program to put a period after the last integer in the output, and I have found ways to do this without using a for loop, but I want to understand why this code doesn't work (it just outputs the larger integer with a period after it).

#include <iostream>

int main()
{
std::cout << "Enter two integers, pressing <ENTER> after each integer." << std::endl;
int num1, num2, lower, upper;
std::cin >> num1 >> num2;
if (num1 > num2)
{
    upper = num1;
    lower = num2;
}
else
    if (num1 < num2)
    {
    upper = num2;
    lower = num1;
    }
    else
        if (num1 = num2)
        {
            upper = num1;
            lower = num1;
        }
std::cout << "All integers between " << lower << " and " << upper << " are:" << std::endl;
for (int val = lower; val <= upper; ++val)
{
    if (val = upper)
    {
        std::cout << val << "." << std::endl;
        ++val;
    }
    else
    {
        std::cout << val << std::endl;
        ++val;
    }
}
return 0;

}

If the two integers entered are 1 and 5, why does this output 5. instead of 1 2 3 4 5.?

The first line defense against using assignment instead of comparison by mistake is to always always use const for any value that does not change. In this case num1 and num2 should be put into const int& right after streaming from std::cin. If an assignment occurs, the compiler will warn you. The upper/lower should also be const and can be initialized with a max and min (standard library functions). E.g.: const upper = max( num1, num2 ); — Zoomulator
– Zoomulator, Commented Jan 14, 2012 at 15:52

score 3 · Accepted Answer · 2012-01-14 15:04:37Z

3

First, you are using the assignment operator = where you should use the comparison operator ==. Second, the for loop already increments val due to the third statement of the for loop (for (...; ...; ++val)). As such, there is no need to increment val from within the body of the loop.

Also, considering that you want to print all results in a single line, you should output a space after each iteration rather than std::endl. Note that the last iteration is an exception as you will want to output a period rather than a space. In the fixed version below, I have used the ternary operator in the loop body to accomplish this.

std::cout << "Enter two integers, pressing <ENTER> after each integer." << std::endl;
int num1, num2, lower, upper;
std::cin >> num1 >> num2;
if (num1 >= num2)
{
    upper = num1;
    lower = num2;
}
else if (num1 < num2)
{
    upper = num2;
    lower = num1;
}

std::cout << "All integers between " << lower << " and " << upper << " are:" << std::endl;
for (int val = lower; val <= upper; ++val)
{
    std::cout << val << ((val == upper) ? "." : " ");
}

edited Jan 14, 2012 at 15:04

answered Jan 14, 2012 at 14:50

user1067737

Sign up to request clarification or add additional context in comments.

1 Comment

lapse Over a year ago

Thank you, I hadn't studied or otherwise known about the ternary operator or the difference between = and ==... this was very helpful.

delannoyk · Accepted Answer · 2012-01-14 14:43:51Z

2

The problem is

if (val = upper)
{
    std::cout << val << "." << std::endl;
    ++val;
}

val = upper means you affect to val the upper value. Put 2 '=' so it will compare instead of affect. Like this

if (val == upper)
{
    std::cout << val << "." << std::endl;
    ++val;
}

answered Jan 14, 2012 at 14:43

delannoyk

1,2231 gold badge13 silver badges23 bronze badges

Comments

Patrick87 · Accepted Answer · 2012-01-14 15:08:14Z

1

Because

if(Val = upper)

Reassigns the value of the Val to be upper. What you meant was

if(Val == upper)

That could be it, anyway.

edited Jan 14, 2012 at 15:08

answered Jan 14, 2012 at 14:43

Patrick87

28.4k3 gold badges41 silver badges75 bronze badges

Comments

Guillaume Paris · Accepted Answer · 2012-01-14 14:44:54Z

0

++val must be present only at for loop declaration level not inside if and else

for (int val = lower; val <= upper; ++val)
   std::cout << val << (val==upper ?  "." : " ") << std::endl;

answered Jan 14, 2012 at 14:44

Guillaume Paris

10.6k15 gold badges79 silver badges155 bronze badges

Comments

MARK · Accepted Answer · 2012-01-14 14:47:57Z

0

There are two errors in your program as mentioned by both delannoyk and Gillaume07. = is the assignment operator in c++ while == is the comparison operator. So almost always if statements are going to have comparison operator. Secondly have increment operation in both for loop statement and the if / else will result in addition of 2 to the counter variable after every iteration.

answered Jan 14, 2012 at 14:47

MARK

2,3724 gold badges26 silver badges44 bronze badges

Comments

Andrei · Accepted Answer · 2012-01-14 14:50:54Z

0

Easier way to do what you did there is to replace this:

for (int val = lower; val <= upper; ++val)
{
    if (val = upper)
    {
        std::cout << val << "." << std::endl;
        ++val;
    }
    else
    {
        std::cout << val << std::endl;
        ++val;
    }
}

with this:

for (int val = lower; val <= upper; ++val)
{
     std::cout << val;

}
cout<<"."<<std::endl;

answered Jan 14, 2012 at 14:50

Andrei

1002 silver badges11 bronze badges

Comments

Dietmar Kühl · Accepted Answer · 2012-01-14 15:03:55Z

0

Others already commented on a number of issues with this code. Here is my pet peeve: You need to check that the input actually was correct! That is, after reading from std::cin you need to check that this was successful. Entering a non-space and non-numeric character would otherwise leave the random garbage which is happens to be stored in your variables as is. The canonical way to read from from an std::istream looks like this:

if (std::cin >> num1 >> num2) {
    ...
}
else {
    possibly an report error here
}

answered Jan 14, 2012 at 15:03

Dietmar Kühl

155k18 gold badges238 silver badges395 bronze badges

Comments

Jia-Luo · Accepted Answer · 2012-01-14 15:18:16Z

0

"=" is used to assign the value of "upper" to "val". when comparing the values of "val" and "upper", the operator to be used should be "==" instead of "=". Therefore, the following code should be:

if (val == upper)
{
    std::cout << val << "." << std::endl;
    ++val;
}

answered Jan 14, 2012 at 15:18

Jia-Luo

3,3235 gold badges19 silver badges17 bronze badges

Collectives™ on Stack Overflow

simple c++ for loop unexpected output

8 Answers 8

1 Comment

Comments

Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

8 Answers 8

1 Comment

Comments

Comments

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related