Java Prime Number help

[wolf2009]

I need help with making this prime number generator in java for univ HW. I'm using BlueJ for writing and testing the code.

I have made the code, but some logic is wrong. The user has to enter the starting number, 2 for HW, and the ending number. Then the code should display all the prime numbers between the two numbers. And it has to be done with loops and if-else statements.

import java.util.Scanner;
public class PrimeNumber
{
public static void main(String [] args)
{
int num1, num2, count, i = 2, prChk = 0 , m = 0;
Scanner keyIn = new Scanner (System.in);


System.out.print("Enter First number : ");
num1 = keyIn.nextInt();

System.out.print("Enter Second number : ");
num2 = keyIn.nextInt();

count = num1;

while(count <= num2)
{
while(i < count)
{
prChk = count % i;
if(prChk == 0)
m = 0;
else if(prChk == 1)
m = 1;
i++;
}
if (m == 0)
System.out.println(count + ": prime");
else if(m ==1)
System.out.println(count + ": not prime");
count++;
}
}
}


What am I doing wrong ?

 

[mundane]

* You need to remember to reset some of the values you use, when re-entering the loops. Given your declarations, it looks like you're coming from a C background - in Java you can declare variable anywhere, as long as you're aware of their scope.
* When testing for primes, as soon as you find a number (>1, <target) which divides evenly into your target, you can declare the test done. You can actually cut that range a bit, although I doubt they're looking for that optimization on your HW.
* Your final analysis appears to be off - if the number is divisible, then it's not prime.

 

[Ken g6]

If you're going to take this approach of finding one prime at a time, I'd suggest splitting out your prime testing code into another method. You can then unit test that method (i.e. test the method alone on a few numbers and see if it returns the right results). Then you know that part is right, and you can focus on making the rest right.

If it was me, I'd use the Sieve of Eratosthenes; but that requires knowledge of arrays, and for this particular instance, a good familiarity with modulo arithmetic. (In other words, the modulus operation doesn't just indicate whether or not count is divisible by i, it also indicates that count-(count%i) and count-(count%i)+x*i are divisible by i.)

 

[degibson]

Always always always put { and } around your if and else blocks. Not only does it help with bad forum mangling of code, it'll save you heartache in the long run.

 

[esun]

Biggest problem I see is that you never reset i. You initialize i to 2, then increment it when you are checking the primality of a number, but never reset it back to 2 when you move to the next number to check.

Oh, and as others have stated, your algorithm is horribly inefficient. But I think at this point you probably want to aim for functionality first.

 

[Cogman]

Originally posted by: degibson
Always always always put { and } around your if and else blocks. Not only does it help with bad forum mangling of code, it'll save you heartache in the long run.

meh, as long as you practice good tabbing it really isn't a big issue. It does make it easier to read on this forum, but elsewhere it doesn't make that big of a difference (IMO).

As for this problem, for one, I would consider a for loop over a while loop. It just looks cleaner IMO.

Another problem I see, m is never checked, but constantly updated. You should either terminate your loop the moment m is 1, or at very least make a check to see if m is 1 before you set it to 0. (preferably terminating the loop for better speed.)

also, consider that other then 2 and 3, no two primes are right next to each other, so you might be better off starting at 3 and adding 2 each time.