Pretty standard. Generates the first n primes, input via a scanner.
package main;
import java.util.Scanner;
public class Prime_Generator {
public static void main(String args[]) {
int n;
int status = 1;
int num = 3;
Scanner scanner = new Scanner(System.in);
System.out.print("First n primes: ");
n = scanner.nextInt();
if (n >= 1) {
System.out.print("First "+n+" prime numbers are: \n");
System.out.println(2);
}
for (int i = 2; i <=n;) {
for (int j = 2; j <= Math.sqrt(num); j++) {
if (num%j == 0) {
status = 0;
break;
}
}
if (status != 0) {
System.out.println(num);
i++;
}
status = 1;
num++;
}
}
}
My question:
- How can I improve the efficiency of this code?
-
\$\begingroup\$ Why use every number from within a certain range when you can instead use every prime within a range? \$\endgroup\$Anthony Pham– Anthony Pham2016年08月05日 18:00:43 +00:00Commented Aug 5, 2016 at 18:00
-
\$\begingroup\$ @PythonMaster How can I do this though? \$\endgroup\$Anonymous– Anonymous2016年08月05日 20:38:04 +00:00Commented Aug 5, 2016 at 20:38
3 Answers 3
Two things:
First, you notice that if I entered "1" as an input, I will get "2," instead of "2".
Second, you check the divisibility of a number n against all numbers from 2 to sqrt(n). This is good, but you can improve it by keeping track of all prime numbers you produce. Whenever you find that p is a prime number, you add it to a list of prime numbers, call it primeList. Now whenever you want to check if n is prime, you can test n against all primes in primeList whose sqrt is less than or equal to n. This way to save on the number of primes to test.
There is a lot of advance theory on finding a prime, but for simple one this is sufficient.
Don't print extras
You have
System.out.print(2 + ", ");
and
System.out.print(num + ", ");
Instead try
System.out.print(2);
and
System.out.print(", " + num);
and at the end
System.out.println(".");
Moving the comma before the current number works better as it is easier to treat the first result differently than the last result. Since the first result is known beforehand (2). Also, you only have to check if one result is first. You have to check all the results to see if they are last.
No even primes past 2
List<Integer> primes = new ArrayList<>();
if (n >= 1) {
primes.add(2);
System.out.print(2);
if (n >= 2) {
primes.add(3);
System.out.print(", " + 3);
}
}
int candidate = 1;
int interval = 2;
while (primes.size() < n) {
interval = 6 - interval;
candidate += interval;
for (int prime : primes) {
if (prime > candidate / prime) {
primes.add(candidate);
System.out.print(", " + candidate);
break;
}
if (candidate % prime == 0) {
break;
}
}
}
System.out.println(".");
This version keeps a list of what primes it has found. It initializes the list with 2 and 3 if it needs that many primes.
After that, it doesn't bother to check any even numbers and skips ever third odd number. This looks like
5, 7, 11, 13, 17, 19, 23, 25,...
Note that the odd numbers it skips (9, 15, 21, ...) are all divisible by three. It does this by alternating between adding 2 and 4 to get the next number. The increment
variable tracks whether the next one should add 2 or 4. Because \6ドル - 2 = 4\$ and \6ドル - 4 = 2\,ドル it alternates between them.
The primes
list keeps track of its own size, so we don't need a separate counter variable (i
in your original code).
I changed the names of num
and j
to candidate
and prime
in this case. I simply find candidate
to be more descriptive than num
. And of course this version only checks for prime
factors, so the name change from j
made sense.
The prime > candidate / prime
expression is checking if prime
is greater than the square root of candidate
. If so, we haven't found a factor smaller than or equal to the square root, so we know that candidate
is prime. The reason for using that form is that many implementations calculate both candidate / prime
and candidate % prime
at the same time. So this minimizes extra calculations.
Separate responsibilities
I left the mixing of calculations and output. That's generally bad style. You should move this into a method and return primes
. Then iterate over primes
to generate the output.
StringBuilder output = new StringBuilder();
for (int prime : generatePrimes(n)) {
output.append(prime);
output.append(", ");
}
output.setLength(output.length() - 2);
output.append('.');
System.out.println(output.toString());
This version will produce the same output as the corrected version of your original.
Or just use the join
version suggested by @ChatterOne.
System.out.println(String.join(", ", generatePrimes(n)) + ".");
Shorter and easier. Not as demonstrative of the powers of a StringBuilder
.
First of all: I guess that
int num = 3;
is testing code that you forgot to remove? AndMath.sqrt(num)
should beMath.sqrt(i)
?To improve efficiency: keep an array, or maybe a hashmap that has a list of non-prime numbers and in your second loop skip the check
if ( num%j == 0 )
if they're in the list. If you use a hashmap it's O(1) time so I'd go for that. Also, see the sieve of Eratosthenes.To not print the last comma, if you don't care about not printing a number as soon as you find it, just keep an array
results
where you push the primes and then print them withString.join(",", results);
. Alternatively, have a look at this question.
-
\$\begingroup\$ When I remove the
int num = 3;
and replaceMath.sqrt(num)
withMath.sqrt(i)
there are many errors that arise. -- As for the array or hashmap thing, could you provide an example of this in use, because I am not as experienced with arrays and hashmaps. \$\endgroup\$Anonymous– Anonymous2016年08月05日 20:51:13 +00:00Commented Aug 5, 2016 at 20:51 -
\$\begingroup\$ In this code,
num
is the number that it is currently checking for primality andi
is the count of primes found. So it is correct to take the square root ofnum
and noti
. \$\endgroup\$mdfst13– mdfst132016年08月06日 21:26:54 +00:00Commented Aug 6, 2016 at 21:26