views:

389

answers:

2

Hello Everybody,

I am suppose to "write a Java program that reads a positive integer n from standard input, then prints out the first n prime number." It's divided into 3 parts.

1st: This function will return true or false according to whether m is prime or composite. The array argument P will contain a sufficient number of primes to do the testing. Specifically, at the time isPrime() is called, array P must contain (at least) all primes p in the range 2 p m . For instance, to test m = 53 for primality, one must do successive trial divisions by 2, 3, 5, and 7. We go no further since 11 > 53 . Thus a precondition for the function call isPrime(53, P) is that P[0] = 2 , P[1] = 3 , P[2] = 5, and P[3] = 7 . The return value in this case would be true since all these divisions fail. Similarly to test m =143 , one must do trial divisions by 2, 3, 5, 7, and 11 (since 13 > 143 ). The precondition for the function call isPrime(143, P) is therefore P[0] = 2 , P[1] = 3 , P[2] = 5, P[3] = 7 , and P[4] =11. The return value in this case would be false since 11 divides 143. Function isPrime() should contain a loop that steps through array P, doing trial divisions. This loop should terminate when 2 either a trial division succeeds, in which case false is returned, or until the next prime in P is greater than m , in which case true is returned.

Then there is the "main function" • Check that the user supplied exactly one command line argument which can be interpreted as a positive integer n. If the command line argument is not a single positive integer, your program will print a usage message as specified in the examples below, then exit. • Allocate array Primes[] of length n and initialize Primes[0] = 2 . • Enter a loop which will discover subsequent primes and store them as Primes[1] , Primes[2], Primes[3] , ……, Primes[n −1] . This loop should contain an inner loop which walks through successive integers and tests them for primality by calling function isPrime() with appropriate arguments. • Print the contents of array Primes[] to stdout, 10 to a line separated by single spaces. In other words Primes[0] through Primes[9] will go on line 1, Primes[10] though Primes[19] will go on line 2, and so on. Note that if n is not a multiple of 10, then the last line of output will contain fewer than 10 primes.

The last function is called "usage" which I am not sure how to execute this! Your program will include a function called Usage() having signature static void Usage() that prints this message to stderr, then exits. Thus your program will contain three functions in all: main(), isPrime(), and Usage(). Each should be preceded by a comment block giving it’s name, a short description of it’s operation, and any necessary preconditions (such as those for isPrime().)

And hear is my code, but I am having a bit of a problem and could you guys help me fix it? If I enter the number "5" it gives me the prime numbers which are "6,7,8,9" which doesn't make much sense.

import java.util.*;
import java.io.*;
import java.lang.*;

public class PrimeNumber {
  static boolean isPrime(int m, int[] P){
    int squarert = Math.round( (float)Math.sqrt(m) );
  int i = 2;
  boolean ans=false;
    while ((i<=squarert) & (ans==false))
    {
  int c= P[i];
      if (m%c==0)
        ans= true;
      else
        ans= false;
      i++;
    }
/*  if(ans ==true)
  ans=false;
  else
    ans=true;
    return ans;
  }  
   *///***********main

  public static void main(String[] args ) {
    Scanner in= new Scanner(System.in);
  int input= in.nextInt();
  int i, j;
  int squarert;
  boolean ans = false;
  int userNum;
  int remander = 0;
  System.out.println("input: " + input);
  int[] prime = new int[input];
  prime[0]= 2;
  for(i=1; i<input; i++){
    j=prime[i-1]+1;
    while(ans!=true)
   {

      ans = isPrime(j,prime);
    j++;}
    prime[i] = j;
   }
  //prnt prime
  System.out.println("The first " + input + " prime number(s) are: ");
  for(int r=0; r<input; r++)
  {System.out.print(prime[r]+ " ");
    if (r%5==0)
      System.out.println();
  }

  }//end of main
}
+1  A: 

Your isPrime method is broken, i should be initialized as 0, not 2.

b0lt
+5  A: 

There are several bugs in the program, along with possible logical/mathematical flaws.
Rather than listing them all, I'll focus fixing/improving the isPrime() method. Also, before getting into these specifics, I'd like to make two suggestions which should help you to handle this kind of situations on your own.

  1. Use a debugger!
    By stepping into the program, and checking whether the values of the variables are the one you would expect, you can identify the specific areas of the program where the logic is wrong.
  2. Test your program at the level of the individual methods
    For example write a small test program which calls isPrime(), several times, each time with some prepared values for the arguments, and check that the returned value is effectively the one expected, for all the test cases. Even if such tests do not cover 100% of the possible cases, they indicate the the routine in question is a priori functional, and allow to then focus on the other methods in the program, with regards to finding bugs etc. There are several test methodologies (unit tests, TDD etc.) and discussing these is beyond the scope of this posting, but even without applying any formal/reusable test framework, you can/should run such ad-hoc tests.

Now... what's wrong with isPrime? Several things...

First off, as listed in the question, this method doesn't return any value. Apparently some code was commented out. Let's re-establish this code.

  static boolean isPrime(int m, int[] P){
     int squarert = Math.round( (float)Math.sqrt(m) );
     int i = 2;
     boolean ans=false;
     while ((i<=squarert) & (ans==false))
     {
       int c= P[i];
       if (m%c==0)
         ans= true;
       else
         ans= false;
       i++;
    }
    if (ans ==true)
      ans=false;
    else
      ans=true;
    return ans;
  }  

Now, there are still other issues. Why starting with i = 2? i is your index into P[], the list of primes. The idea behind the method is try and divide m, the number being tested for primality, by all the prime numbers smaller than m's square root, and too deem m as a prime number if no such division produce a zero remainder (m modulo some_prime equals zero).
You should therefore start with P[0] not P[2]. I'm guessing you confused the subscript with the expected value (P[0] is initialized with the first prime, 2).

A similar confusion is done a bit later, when the loop continues as long as
i <= squarert & ...
The test should really be
P[i] <= squarert & ...
i.e. you want to try divisions as long as the possible prime factor is less or equal to the square root, rather than try and divide with the nth first primes (where n is the square root of the test integer). For example, when testing say 13, which square root is 3.6... , you need to test with primes smaller or equal to 3, not for the first 3 primes.

The above also bring another flaw in the logic. You need to use Math.floor() rather than Math.round(). The problem with Math.round is that it may round the square root value to the next integer (with our 13 example, it would provide 4 rather than 3). This btw is not really a bug, but rather an inefficiency, allowing for one extra, unnecessary test in some cases.

Beyond the bugs mentioned above, and without trying to introduce alternative logic etc, the method could be improved in at least in two ways:

  • why bother setting ans to false in the "else" case. It is already false (by initialization)
  • why not reverse the logic of ans, to avoid having to "flip it", before exiting the method.

With these two improvements, and the bug fixes mentioned, the method would become something like:

  static boolean isPrime(int m, int[] P){
     int squarert = Math.floor( Math.sqrt(m) );  // btw sqrt returns a double no need to cast to float
     int i = 0;
     boolean ans = true;  // assume m will be a prime
     while ((P[i] <= squarert) & (ans == true))
     {
       int c = P[i];
       if (m % c != 0)
         ans= false;
       i++;
    }
    return ans;
  }  

Finally, and while I respected the original logic whereby the loop continuation test includes a test on ans having some value, I'd like to suggest the use of the "break" statement instead. Sorry to add so much info at at time your focus is on bugs ;-)... The loop could become something like

     while (P[i] <= squarert)  // no more test on ans
     {
       int c = P[i];
       if (m % c != 0)
       {
         ans = false;
         break;      // this causes the loop to terminate
       }
       i++;
     }
mjv
That you wrote this answer, and wrote it so well is almost religiously nice of you mjv.
Lars Andren