Why is the answer printed twice?
I have created a program that returns the product abc, where a, b, c are Pythagorean triples up to 1000. The program prints out the correct answer, but it does it twice. I was curious as to why this is so. After playing with it a bit, I found out that it prints when a = 200 b = 375 c = 425. And 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;
}
a source to share
The gap in this case will break out of the cycle c
, and not b
and a
.
A quick fix is that you don't get repetitions by running each variable greater than or equal to the previous one (so b
no less a
and c
no less b
).
In addition, you can completely get rid of the loop altogether c
, since a,b
there is only one value for a given pair c
(if a + b + c > 1000
, and in this case there is 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 this is to reduce the total number of cycles from a billion (short) to about half a million, and hence reduce it by about 99.95% - this should be a little faster :-)
And maybe make it faster with Jerry Coffin's suggestion (and inline suggestion to the compiler), the complete 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) and the original takes about 2.772 seconds (ten samples each). Not that it really matters unless you run it many, many times, of course.
The output of this code is as expected:
200 375 425 31875000
a source to share
The reason for this is that you are only exiting the inner loop (for c). The outer loops continue to run and re-enter the inner loop, again satisfying the conditions. There are many values that add up to 1000 and you will catch them - you caught 2 as your fingerprint indicates. You can use "return" instead of break if you only want the first combination of values.
As for the "code block", I'm not sure what you mean. You already seem to know how to write functions. If you mean a scoped block, then you simply enclose the code of concern in curly braces → {}
eg.
{int i = 0; i ++; }
a source to share