Why does the answer print out twice?

Question

I made a program that returns the product a*b*c where a,b,c are pythagorean triples and add up to 1000. The program does output the correct answer but does it twice. I was curious as to why this is so. After playing around with it a bit I found out that it prints out when a = 200 b = 375 c = 425. And once again when a = 375 b = 200 c = 425.

bool isPythagTriple(int a, int b, int c);

int main()
{

    for(int a = 1; a < 1000; a++)
    {
        for(int b = 1; b < 1000; b++)
        {
            for(int c = 1; c < 1000; c++)
            {
                if( ((a+b+c)==1000) && isPythagTriple(a,b,c) )
                {
                    cout << a*b*c << " ";
                    break;
                }
            }
        }
    }

    return 0;
}

bool isPythagTriple(int a, int b, int c)
{
    if( (a*a)+(b*b)-(c*c) == 0 )
        return true;
    else
        return false;
}

Answer 1

This is how break and continue work - break only exits the inner-most loop. Read the discussion on this question for some alternatives to this.

Answer 2

Break, in this case, will only break out of the c loop, not the b and a ones.

A quick fix is to ensure you don't get repeats by starting each variable greater than or equal to the previous (so b is never less than a and c is never less than b).

In addition, you can actually get rid of the c loop altogether since there's only one value of c that is valid for a given a,b pair (unless a + b + c > 1000 in which case there are none). I would try something like:

for (int a = 1; a < 1000; a++) {
    for (int b = a; b < 1000; b++) {
        int c = 1000 - a - b;
        if (c >= b) {
            if (isPythagTriple (a,b,c)) {
                cout << a << " " << b << " " << c << " " << a*b*c << std::endl;
            }
        }
    }
}

The overall effect of that is to reduce the total loop count from a billion (short scale) to about half a million hence reducing it by about 99.95% - that should be a tiny bit faster :-)

---

And potentially making it faster with Jerry Coffin's suggestion as well (and an inline suggestion to the compiler), a full program:

#include <iostream>

inline bool isPythagTriple(int a, int b, int c) {
    return a * a + b * b == c * c;
}

int main() {
    for(int a = 1; a < 1000; a++) {
        for(int b = a; b < 1000; b++) {
            int c = 1000 - a - b;
            if (c >= b) {
                if (isPythagTriple(a,b,c)) {
                    std::cout << a << " " << b << " " << c << " "
                        << a*b*c << std::endl;
                }
            }
        }
    }
    return 0;
}

which takes 0.004 seconds on average (system + user) on my box, with the original taking about 2.772 seconds on average (ten samples each). Not that it really matters unless you're running it many, many times, of course.

The output of that code is, as expected:

200 375 425 31875000

Answer 3

The reason this happens is because you only break out of the inner loop (for c). The outer loops carry on running and re-enter the inner loop, meeting the conditions again. There are numerous values that add to 1000 and you are catching some of them - you have caught 2, as your print indicates. You could use "return" instead of break if you want only the first combination of values output.

As for the "code block" I am not sure what you mean.. You already seem to know to write functions. If what you mean is a scope block, then you simply enclose the code of concern in curly braces -> { }

E.g.

{ int i = 0; i++; }

Answer 4

To prevent multiple orderings of the solutions, make sure that c >= b >= a. You can do this by changing the lower bounds:

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

Answer 5

Just for what it's worth, I'd write this function:

bool isPythagTriple(int a, int b, int c)
{
    if( (a*a)+(b*b)-(c*c) == 0 )
        return true;
    else
        return false;
}

More like this:

bool isPythagTriple(int a, int b, int c) { 
    return a*a+b*b==c*c;
}