Java Help

[namarino]

import java.util.Scanner;public class Application {	public static void main(String[] args) {		Scanner scan = new Scanner(System.in);		int userNum = 0;		int a = 0;		int b = 0;		double c = Math.sqrt((a*a) + (b*b));				System.out.print("Please enter a number: ");		userNum = scan.nextInt();				for(int num = 1; num < userNum; num++){			a = num;			for(int num2 = 1; num2 < userNum; num2++){				b = num2;				if(c == Math.round(c) && ((a*a) + (b*b)) == (c*c)){					System.out.println(a + " " + b + " " + c + " ");				}			}		}	}}				

Hey guys, I'm writing a program for my intro to computer science II class. Essentially the user inputs a number and the program spits out all the pythagorean triples whose a and b values are less than or equal to that value. This is what I have so far but I cannot get it to bloody print anything and I can't figure out why. Can someone help? Essentially I can't get it to enter into that conditional statement.

[MrSuperb]

if(c == Math.round(c) && ((a*a) + (b*b)) == (c*c))

probably the problem. never check doubles for equality.

[Evil Genius Jr.]

It's something to do with java not being able to accurately see if doubles are equal. It's a pretty complex issue which I don't know much about. Try removing the 

c == Math.round© 

[namarino]

probably the problem. never check doubles for equality.

What do you mean exactly?

[namarino]

 

It's something to do with java not being able to accurately see if doubles are equal. It's a pretty complex issue which I don't know much about. Try removing the 

c == Math.round(c) 

so what should I replace that with because that's a key component of the program?

[WiiManic]

Isn't C always zero in this case? Like you set it at the start, but at that point a and b are both also 0.

You never update, so it stays at 0.

 

So when you do ((a*a) + (b*b ) == (c*c)), you are doing ((1) + (1)) == (0)). Which will never be correct.

[DavidTheWin]

You never assign to c. You do an initial assignment when you set a and b to 0, also setting c to 0. You never set it after that when you actually get the values of a and b.

[Evil Genius Jr.]

Try this, just moved the c declaration.

import java.util.Scanner;public class Application {	public static void main(String[] args) {		Scanner scan = new Scanner(System.in);		int userNum = 0;		int a = 0;		int b = 0;				System.out.print("Please enter a number: ");		userNum = scan.nextInt();				for(int num = 1; num < userNum; num++){			a = num;			for(int num2 = 1; num2 < userNum; num2++){				b = num2;				double c = Math.sqrt((a*a) + (b*b));				if(c == Math.round(c) &&((a*a) + (b*b)) == (c*c)){					System.out.println(a + " " + b + " " + c + " ");				}			}		}	}}			

[namarino]

You never assign to c. You do an initial assignment when you set a and b to 0, also setting c to 0. You never set it after that when you actually get the values of a and b.

 

 

Isn't C always zero in this case? Like you set it at the start, but at that point a and b are both also 0.

You never update, so it stays at 0.

 

So when you do ((a*a) + (b*b ) == (c*c)), you are doing ((1) + (1)) == (0)). Which will never be correct.

RIGHT! haha I just saw that in the debugger....So what exactly do I have to do to fix that?

[Evil Genius Jr.]

Here's also an explanation on doubles for future reference: https://stackoverflow.com/questions/1088216/whats-wrong-with-using-to-compare-floats-in-java

Doesn't seem to be the issue here though.

[namarino]

 

Try this, just moved the c declaration.

import java.util.Scanner;public class Application {	public static void main(String[] args) {		Scanner scan = new Scanner(System.in);		int userNum = 0;		int a = 0;		int b = 0;				System.out.print("Please enter a number: ");		userNum = scan.nextInt();				for(int num = 1; num < userNum; num++){			a = num;			for(int num2 = 1; num2 < userNum; num2++){				b = num2;				double c = Math.sqrt((a*a) + (b*b));				if(c == Math.round© &&((a*a) + (b*b)) == (c*c)){					System.out.println(a + " " + b + " " + c + " ");				}			}		}	}}			

That did the trick! Thanks a lot! Now the only thing is it prints doubles of things because they are in different orders. Is there a way that I can stop that from happening?

[Evil Genius Jr.]

So for that you want a to always be less than b or vice versa. I do have the solution but it's definitely more satisfying if you find it so I'll give you a hint. You have to modify the second for loop.

[DavidTheWin]

Other stylistic things:

 

There's no need to declare and initialise a, b and userNum at the start. It's all about scope, since you don't need a or b until the loop you only need to declare them in the loop and they work perfectly as the loop iterators since you only assign the loop iterator to them.

I also replaced the tabs with 4 spaces. This is more a C# standard than a Java standard but I think it equally applies to both. Spaces allow for more flexible spacing and I think the smaller indentations here make it subjectively more readable, you don't have to go quite as far to the right. 

I also moved the opening braces to the new line. Again this is more a C# standard than a Java standard (I think the Java standard actually recommends same line braces) but to me this is a code style that should be applied to any C-like languages. Following the scope of a for loop/if statement/method/class or whatever is much easier with a new line brace. In a similar vein, don't omit the braces in cases where the language will allow you, e.g. single line for and if loops. For example the loop could be written like:

        for(int a = 1; a < userNum; a++)            for(int b = 1; b < userNum; b++)            {                double c = Math.sqrt((a*a) + (b*b));                if(c == Math.round(c) && ((a*a) + (b*b)) == (c*c))                    System.out.println(a + " " + b + " " + c + " ");            } 

This is BAD. If you want to extend the outer for loop or the inner if statement you have to add the braces. If you forget then your code will still compile but be subtly wrong which is worse than not compiling. Outright failure under incorrect conditions is better than the program silently working wrong.

 

Finally, I just cleaned up the spacing, e.g. new line after imports and made the spacing CONSISTENT around the && in the if statement. Consistency with spacing is key.

import java.util.Scanner;public class Application{    public static void main(String[] args)    {        Scanner scan = new Scanner(System.in);		        System.out.print("Please enter a number: ");        int userNum = scan.nextInt();		        for(int a = 1; a < userNum; a++)        {            for(int b = 1; b < userNum; b++)            {                double c = Math.sqrt((a*a) + (b*b));                if(c == Math.round(c) && ((a*a) + (b*b)) == (c*c))                {                    System.out.println(a + " " + b + " " + c + " ");                }            }        }    }}

[wolfsinner]

I'd like to subscribe to the "don't use floating point variables" suggestions. The reason why your code has absolutely no problems is because there's no arithmetic going on that will mess up your decimal point approximation - you only care about the cases where there is no decimal value. Also, Math.round actually returns a long, which means there's some auto-casting magic going on there you don't know about.

 

Just don't use floating point when you need to perform arithmetic on values and compare them later for equality. There are safer (albeit less efficient) alternatives. It'll save you a sea of headaches.

If you are curious about why that's not cool, I wrote here about the problem.

[namarino]

So for that you want a to always be less than b or vice versa. I do have the solution but it's definitely more satisfying if you find it so I'll give you a hint. You have to modify the second for loop.

So basically what I did was changed it so that b is always less than a  and then I changed the print statement to print b first, then a because it would print out 4 3 5  instead of 3 4 5. I think that did the trick. Is that the solution that you had in mind?

[Evil Genius Jr.]

Yup! I just changed it to b<a instead of b<usernum.