CODEWITHSUNDEEP
c++
rEgonicS
rEgonicS

Reputation: 211

Why does the answer print out twice?

I made a program that returns the product abc 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;
}

Upvotes: 2

Views: 1117

Answers (5)

paxdiablo
paxdiablo

Reputation: 882028

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

Upvotes: 8

Jerry Coffin
Jerry Coffin

Reputation: 490328

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;
}

Upvotes: 9

msw
msw

Reputation: 43507

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++) {

Upvotes: 0

filip-fku
filip-fku

Reputation: 3265

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++; }

Upvotes: 0

D.Shawley
D.Shawley

Reputation: 59573

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.

Upvotes: 2

Related Questions