montecarlo76
montecarlo76

Reputation: 31

Please assist: "if" statement in C++

The following code prints the same result - twice (!) (using Microsoft Visual C++ 2010 IDE). I also printed the final values of each variable to see what was going on and there are in effect two sets of values that satisfy the if statement condition.

My question is since the instruction was break; if the condition evaluated TRUE, can anyone kindly explain how/why I am getting both results when I didn't ask for that (not that it's a bad thing, just trying to understand) ? Is this part of the if construct or does it have something to do with the loops ? It seems as though something knows to return multiple solutions if the condition evaluates to TRUE more than once, I just don't get how it's able to do that when the instructions don't explicitly say to do that (unless there is something built-in that I don't know of).

Basically, why doesn't the loop end at break; once the condition is met or am I thinking about this the wrong way ?

Again, if anyone knows or if I am missing something basic here please let me know ! I'm new to C++ so just trying to learn, thank you in advance.

Here is the code:

#include "stdafx.h"
#include <iostream>     

int main()
{
    for (int a = 1; a < 500; ++a)
    {
        for (int b = 1; b < 500; ++b)
        {
            for (int c = 1; c < 500; ++c)
            {
                if ((a + b + c) == 1000 && ((a*a + b*b) == (c*c)))
                {
                 cout << "The product abc = " << a*b*c << endl << "a = " << a << ", b = " << b << ", c = " << c << endl;
                 break;
                }
            }
        }
    }
    cout << endl << "Loop terminated";

    char d;
    cin >> d;
    return 0;
}

The console output is the following:

The product abc = 31875000

a = 200, b = 375, c = 425

The product abc = 31875000

a = 375, b = 200, c = 425

Loop terminated

Upvotes: 3

Views: 286

Answers (7)

gnasher729
gnasher729

Reputation: 52530

Your code will be awfully slow if you try to check a slightly larger range of numbers. You try actually all values for c from 1 to 500 and check whether a + b + c = 1000. It is obvious that the sum is only 1000 if c = 1000 - a - b. So you can just write

int c = 1000 - a - b;
if ((c >= 1 && c < 500) && (a*a + b*b == c*c)) ...

This will run about 500 times faster...

Now you don't like that both a = 200, b = 375 and a = 375, b = 200 are printed. You might think about a break statement, but that will introduce a bug: There are plenty of cases where there are multiple solutions that are not trivially connected like this one here.

What you want is to avoid printing solutions where a > b, because if (a, b, c) is a solution, then (b, a, c) is also a solution. An easy way to do this is to write

for (int b = a; b < 500; ++b) ...

When a = 375, only values 375 to 499 are checked for b, which avoids printing a = 375, b = 200.

Upvotes: 0

user180247
user180247

Reputation:

As a number of people have said already, break only breaks out of the innermost loop.

Some languages provide a fix for this problem. Ada definitely has a way to name each loop, and its nearest equivalent to break, exit when <condition>;, can also be written exit <loopname> when <condition>;. IIRC C# may also have special syntax for this.

In C++, your options are...

  1. Use goto rather than break. Put the target label just after the outermost loop.

  2. throw an exception to break out of the loop, with all your nested loops nested inside the try block.

  3. Don't break out of the loop at all. Instead, have a flag, done or similar, to indicate when you're finished. Check for that in the conditions for all loops.

  4. As Jerry Coffin said, use move the loops to a separate function and use return (don't know how I missed that one!).

Purists will probably prefer 3, with an option on 2. In particular, the only option that doesn't break the "single exit principle" from structured programming is (3), but break also breaks that principle anyway. Personally, I'd prefer 1 as it generates less clutter so (provided the function you use it in is small, and the goto is visually obvious) it's more readable.

Using goto is generally avoided, with good reason. A lot of people ban them completely, which is less justified, but those people will probably consider break and continue to be "hidden gotos" and ban those too (except for break in switch statements, of course). Anyway, as a result, you may not even know that goto is possible.

There's a description of the syntax for goto here - basically goto <labelname>; and labelname:.

Upvotes: 0

masoud
masoud

Reputation: 56479

The break you used just breaks most inner for loop not all loops.

Use a flag and and it to for conditions:

bool found = false;
for (int a = 1; a < 500 && !found; ++a)
{
    for (int b = 1; b < 500 && !found; ++b)
    {
        for (int c = 1; c < 500 && !found; ++c)
        {
            if ((a + b + c) == 1000 && ((a*a + b*b) == (c*c)))
            {
             cout << "The product abc = " << ....
             found = true;
            }
        }
    }
}

or even you can use a goto politely !

for (int a = 1; a < 500; ++a)
{
    for (int b = 1; b < 500; ++b)
    {
        for (int c = 1; c < 500; ++c)
        {
            if ((a + b + c) == 1000 && ((a*a + b*b) == (c*c)))
            {
             cout << "The product abc = " << ...
             goto break_all;
            }
        }
    }
}
break_all:

"This is the last remaining stronghold for the use of goto"

Upvotes: 0

fatihk
fatihk

Reputation: 7919

break statement only breaks the innermost loop and you may need to use a flag to exit other loops in the case of a break

#include "stdafx.h"
#include <iostream>     

int main()
{
bool flag = false;
for (int a = 1; a < 500; ++a)
{
    for (int b = 1; b < 500; ++b)
    {

        for (int c = 1; c < 500; ++c)
        {
            if ((a + b + c) == 1000 && ((a*a + b*b) == (c*c)))
            {
             cout << "The product abc = " << a*b*c << endl << "a = " << a << ", b = " << b << ", c = " << c << endl;            
             flag = true;
             break;
            }
        }

        if(flag)
        { break; }
    }

    if(flag)
    { break; }
}
cout << endl << "Loop terminated";

char d;
cin >> d;
return 0;

}

Upvotes: 0

Vinoj John Hosan
Vinoj John Hosan

Reputation: 6853

In your code you just applied only one break. So the inner most for loop will be broken and other outer loop will continue.

So you have define flag variable to break all loops.

#include "stdafx.h"
#include <iostream>  
    int main()
    {
        bool flag = 0; //Set the flag
        for (int a = 1; a < 500 && !flag; ++a) //Check flag condition
        {
            for (int b = 1; b < 500  && !flag; ++b) //Check flag condition
            {
                for (int c = 1; c < 500 && !flag; ++c) //Check flag condition
                {
                    if ((a + b + c) == 1000 && ((a*a + b*b) == (c*c)))
                    {
                     cout << "The product abc = " << a*b*c << endl << "a = " << a << ", b = " << b << ", c = " << c << endl;
                     flag= 1; //Set the flag true
                     break;
                    }
                }
            }
        }
        cout << endl << "Loop terminated";

        char d;
        cin >> d;
        return 0;
    }

Upvotes: 0

Jerry Coffin
Jerry Coffin

Reputation: 490028

Your break only breaks out of the inner loop, so the outer loop continues to execute.

One possibility would be to move that code into a separate function, and return from the function when you find the match the first time. Then (unless you call the function again) execution of that code will cease completely.

I'd also eliminate the loop for c entirely. The only meaningful value for c is 1000 - (a+b), so you might as well just compute it directly instead of looping through 500 different values to find it.

Upvotes: 5

Floris
Floris

Reputation: 46365

So - you have heard that break; is not the solution - but what is?

If you make your limit (currently 500) for each loop a variable, say

int myLim = 500;

And replace each of the for loop conditions with

for (int a = 1; a < myLim; ++a)
    {
        for (int b = 1; b < myLim; ++b)
        {
            for (int c = 1; c < myLim; ++c)
            {

Then instead of break you say myLim = -1 - and you will break out of all the loops.

Note - some compilers will frown upon you setting a=500 and b=500 when the if condition is satisfied, since you are modifying a loop variable; but none will mind if you change the condition. It's a bit cleaner than adding a flag, but still a bit of a hack.

Upvotes: 0

Related Questions