3

I'm having trouble writing a program that finds the prime numbers between 2-50. My program right now finds "mostly" prime numbers but some non-prime numbers are included. I know there are way more effective methods and strategies for finding prime numbers but I'm trying to first run through every possibility before i move onto more efficient and effective strategies. The way the program is written now if it does find a prime number it prints it multiple times when I only want it to be printed once. Is my program conceptually correct or is my rationale flawed. Why does my program mostly find prime numbers but throw in some non prime numbers? Why does my program print the prime numbers multiple times?

Here is my approach to the problem.

  1. create a for loop to represent potential prime numbers between 2-50. Represent these potential prime numbers by the variable "i". This loop will count down from 50.

  2. Since a number is prime only if there are no other divisible numbers besides itself and 1, i want to create a inner for loop to divide i by every possible number between 2 and i -1 to see if any of these numbers divides evenly into i. Represent these possible divisors by the variable j. If at any point j does divides evenly into i it's not a prime number so I want my inner loop to exit.

  3. If i gets divided by all the numbers of j and there are no numbers that divide evenly into i then that number if prime and want to print that number.

*

import acm.program.*;
public class PrimeNumber extends ConsoleProgram{
public void run(){

  for (int i =50; i >= 2; i--){
    for (int j= 2; j < i-1; j++){

        if (i % j >= 1){
         println(i);
         }else{
         if (i % j == 0) break;
                }
              } /*end of inner loop */  
           } /* end of for loop */

       } /* end of run method */
     } 
Jessica M.
  • 1,451
  • 12
  • 39
  • 54
  • 2 and i -1 this is too much, you can check divisions between 2 and sqrt(i). – cerkiewny May 24 '13 at 02:00
  • For this algorithm to work, then i%j would have to be >= 1 for ALL the inner loop cases, not just one. As cerkiewny points out, you can reduce the loop iterations by just going to sqrt(i). – lurker May 24 '13 at 02:00
  • i++ in the inner loop must be a typo? – Radio- May 24 '13 at 02:02
  • hehe ... comments are faster than answers ... beat me to it. – rolfl May 24 '13 at 02:04
  • @Radio- No, not a typo. I want the outer loop to count down and the inner loop to count up. So for example to test 50 i want 50 % 2...3...4.. all the way up to 50 % 49. – Jessica M. May 24 '13 at 02:04

8 Answers8

2

You made 2 mistakes. The first is explained in the comment by @Thomas and the second is explained in the comment by @rolfl. I corrected them both here:

public class PrimeNumber extends ConsoleProgram {
    public void run() {
        for (int i = 50; i >= 2; i--) {
            boolean isPrime = true;
            for (int j = 2; j < i-1; j++) { //increment "j" not "i"
                if (i % j == 0) {
                    isPrime = false;
                    break;
                }
            }
            if (isPrime) System.out.println(i);
        }
    } 
}

Note: I verified the solution using this code (save in your IDE as PrimeNumber.java):

public class PrimeNumber {
    public static void main (String args[]) {
        for (int i = 50; i >= 2; i--) {
            boolean isPrime = true;
            for (int j = 2; j < i-1; j++) { //increment "j" not "i"
                if (i % j == 0) {
                    isPrime = false;
                    break;
                }
            }
            if (isPrime) System.out.println(i);
        }
    } 
}

Edit: For your understanding, your main problem is the logic here:

for (int j= 2; j < i-1; j++) {
    if (i % j >= 1) {
        println(i);

You are printing i after only checking one possibility.

For example, take i = 7. You have to test i % j for j = 6, 5, 4, 3, and 2 before you can say that i is prime. You can't just test i % j for j = 6, as you have done. To do so, your println statement should come after the for loop, not nested inside of it, so you can test all the possible values of j first.


Edit 2: In response to

Coincidentally enough, the 1st part of the assignment is to write a predicate method that returns true if the number is prime and false if it is not prime using the strategy of brute-force. The 2nd part of the assignment is to find a more efficient way and reworking the 1st part of the assignment to be more efficient. I was trying to solve the problem using just using two for loops to see if i could do it. Can it be done with just two for loops w/o label and continue since my book has not covered that yet?

Try something like this:

public boolean isPrime(int number) {
    for (int i = 2; i < number; i++) {
        if (number % i == 0) {
            return false; //not prime
        }
    }
    return true; //prime
}
Community
  • 1
  • 1
TheBlackKeys
  • 852
  • 1
  • 9
  • 15
  • thanks a lot. I'm still studying and experimenting with all the answers. Your answer makes a lot of sense. – Jessica M. May 24 '13 at 02:44
  • thanks for all the help. I really appreciate it. Your answers have really helped me a lot. – Jessica M. May 24 '13 at 03:46
  • You're welcome. And as far as part 2 goes, off the top of my head you only really need to check that the number is not even and then take the mod (%) of the odd numbers. – TheBlackKeys May 24 '13 at 04:21
1

You correctly observed that if a number i can be evenly divided by a number j, then i % j == 0.

However, you're printing i every time you find a case where i % j >= 0 -- that doesn't mean however thati is prime, because there could be some other j that i can be evenly divided by.

Instead, you should first go through all of your j, and only if none of them gives you == 0, should you consider i prime. You could use a boolean variable isPrime that is initially true, but that the inner for-loop sets to false as soon as it finds a j by which i can be evenly divided.

Thomas
  • 17,016
  • 4
  • 46
  • 70
  • that's the second part of the assignment. To later use a predicate method that is true of the number is prime and false if it is not. I have not worked out the fist part of finding prime numbers through the brute-force method yet. – Jessica M. May 24 '13 at 02:28
  • can this problem be solved with just two for loops or does a boolean value have to be involved? – Jessica M. May 24 '13 at 02:56
  • You can do it with just two for-loops, if you use `continue` in the inner loop together with a label for the outer loop. That would be like using GOTO. Are you sure that the second part of your assignment implies that you cannot use a boolean variable in the first part? – Thomas May 24 '13 at 03:09
  • Coincidentally enough, the 1st part of the assignment is to write a predicate method that returns true if the number is prime and false if it is not prime using the strategy of brute-force. The 2nd part of the assignment is to find a more efficient way and reworking the 1st part of the assignment to be more efficient. I was trying to solve the problem using just using two for loops to see if i could do it. Can it be done with just two for loops w/o label and continue since my book has not covered that yet? – Jessica M. May 24 '13 at 03:22
  • I don't think it can be done. But it doesn't sound to me as if you're supposed to use only two for-loops and nothing else. Note that a boolean variable is not a predicate method. I suspect that you're supposed to write a predicate method `boolean isPrime(int n)` that basically does what your inner for loop does currently, but that you're allowed to use a boolean variable inside that method. Then the outer for-loop could call that predicate method instead of having its own inner loop. Sorry, not sure I'm being very helpful. – Thomas May 24 '13 at 03:39
  • I think you are being asked to write a method that returns true if the number is prime, but what you are writing is a method that returns void. See the edit to my original response for an example. – TheBlackKeys May 24 '13 at 03:41
  • @Thomas I appreciate your help, advice and observations. It's really help me work out this problem. Thanks a lot! – Jessica M. May 24 '13 at 21:09
1

There is a bug in your second loop (the inner loop).

You should be incrementing j and not i.... i.e. the innner loop should be

for (int j= 2; j < i-1; j++){

and not

for (int j= 2; j < i-1; i++){
rolfl
  • 17,539
  • 7
  • 42
  • 76
0

This if statement:

if (i % j >= 1){
         println(i);

Will print every number that is not divided by j number, and the numbers you want to print are not those that will get a value of division different from 0 from one substract but for ALL substracts.

Also the faster solution is taking the Boolean array of potentially prime numbers and looping like this.

for ( i = 2; i < sqrt(maxvalue); i ++){
            for(j = 1; j < sqrt(maxval); j++){
                  potentialy_prime[i*j]=false
            }
       }
for (potentialy_prime){
    if(potentialy_prime[i]==true)
            print(i "is prime")
}
cerkiewny
  • 2,761
  • 18
  • 36
0

Check out dydx's answer on this question: https://stackoverflow.com/questions/13437162/java-finding-prime-numbers. Or this question: Prime number calculation fun or this one Getting the prime numbers, etc...

Community
  • 1
  • 1
Matt Becker
  • 2,338
  • 1
  • 28
  • 36
0

There are 3 problems with your code :

  • In the inner loop, you are increasing i instead of increasing j.

  • You print the number as soon as i % j >= 1, but maybe the next j will divide i, and thus i isn't prime.

  • You should stop the inner loop at sqrt(i) to avoid useless tests.

Here is a corrected version of the code :

OUTER: for (int i = 50; i >= 2; i--) {
    for (int j = 2; j <= (int) Math.sqrt(i); j++) {
        if (i % j == 0)
            continue OUTER;
        } /* end of inner loop */
    println(i);
} /* end of for loop */
Florent Bayle
  • 11,520
  • 4
  • 34
  • 47
0

This code should print out the prime numbers. Initial assumption is that every integer >= 2 is prime(true in boolean array). while traversing through the array, if the number is found to be prime, cross out multiples of that number by setting them false in boolean array ( as they are not prime). At the end the numbers having true in boolean array will be prime numbers only.

Time complexity : O(n log(log n))

public class PrimesImprovement
{
    public static void main(String[] args)
    {  
        System.out.println("Printing primes from 1 to 100");
        int i;
        int j;
        //boolean prime;
        boolean[] prime_arr=new boolean[100];
        Arrays.fill(prime_arr,true); // Assumption
        prime_arr[0]=prime_arr[1]=false;  // As 0 and 1 are not prime numbers
        for ( i=2;i<prime_arr.length;i++) 
        {

            if(prime_arr[i])
            { 
                for ( j=2;i*j<prime_arr.length;j++) 
                {
                    prime_arr[i*j]=false;
                }
            }
        }

        for( i=0; i<prime_arr.length; i++)
        {
            if(prime_arr[i])
            {
                System.out.print(i+" ");
            }
        }

        System.out.println();
    }
}
Archit Garg
  • 1,012
  • 8
  • 16
0

Sorry nitpicking here, but why on earth are you all using post-increment operators in for loops? Do you realize the difference between ++i and i++?

int var1 = 5;
int var2 = 5;

int result1 = ++var1;  // Pre-increment: first increment var1 to 6, then assign to result1
int result2 = var2++;  // Post-increment: first assign var2 to result2, then increment var2 to 6

System.out.println("Pre-increment: " + result1 + ", var1: " + var1);  // Output: Pre-increment: 6, var1: 6
System.out.println("Post-increment: " + result2 + ", var2: " + var2);  // Output: Post-increment: 5, var2: 6
Karoly Nyisztor
  • 3,505
  • 1
  • 26
  • 21