3

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

Community
  • 1
  • 1
montecarlo76
  • 31
  • 1
  • 3
    `break` breaks out of the innermost for loop only. – Floris Apr 22 '13 at 05:04
  • Thanks, yea, I'm stupid... – montecarlo76 Apr 22 '13 at 05:16
  • Though my official answer is garbage and soon deleted, look at this [answer](http://stackoverflow.com/questions/14863511/find-unique-pythagorean-triples/14865022#14865022) I submitted to a simular question. It's quite different but the overall result is eliminating duplicates in a similar situation. Also notice the use of a boolean to control the looping. – ChiefTwoPencils Apr 22 '13 at 05:20
  • `break` is the wrong solution. `a` and `b` are perfectly interchangeable in the equations you're solving. You'll get duplicate solutions unless you limit `b` from above or below by `a`. Try it with the second loop being `for (int b = a; b < 500; ++b)` and the duplicates where `a` and `b` are interchanged will disappear. – Olathe Aug 18 '13 at 15:55

7 Answers7

5

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.

Jerry Coffin
  • 476,176
  • 80
  • 629
  • 1,111
  • A more hackish way is a boolean isdone flag which you check in all the loop conditions. – Antimony Apr 22 '13 at 05:05
  • Ahhh.... thanks so much guys !!! I feel so relieved (and like an idiot), I completely forgot about that. I also just realized there was a third condition missing in the `if` statement, I should have included a – montecarlo76 Apr 22 '13 at 05:15
  • 1
    @montecarlo76 - if you have `a –  Apr 22 '13 at 05:34
  • 1
    I like the separate function idea. In C++11 there's a new pattern like that where you just use a lambda: quite clean, doesn't need a separate declaration/definition, allows variable capture too, what's not to like? ;) A bit OT but this lambda pattern is also very nifty for const variables that require complex initialization. – syam Apr 22 '13 at 05:36
0

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;

}

fatihk
  • 7,789
  • 1
  • 26
  • 48
  • if the flag is checked at end of the for loop then it can be optimized and will execute two loops less then this... Just curious about the optimization otherwise good logic works fine... – bikram990 Apr 22 '13 at 05:25
0

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"

Community
  • 1
  • 1
masoud
  • 55,379
  • 16
  • 141
  • 208
  • 1
    I suppose I'd like to -1 for *any* goto as advice (imo), but I'm going to +1 because I think it's the first time I've ever seen anyone do it. – ChiefTwoPencils Apr 22 '13 at 05:27
0

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.

Floris
  • 45,857
  • 6
  • 70
  • 122
0

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;
    }
Vinoj John Hosan
  • 6,448
  • 2
  • 41
  • 37
0

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:.

Community
  • 1
  • 1
0

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.

gnasher729
  • 51,477
  • 5
  • 75
  • 98