patternjavaModerate
Sum of First N Primes
Viewed 0 times
firstprimessum
Problem
Task: Print sum of first 1000 primes.
I'm trying to adjust my programming style to a "people first" paradigm. While any and all feedback is welcome, that is the focus.
public class PrimeSummation {
public static void main(String[] args) {
System.out.println(sumFirstPrimes(1000));
}
// Calculates the total of the first n primes
public static int sumFirstPrimes(int n) {
/* Since 2 is the only even prime it is pre-included;
affords convenience of skipping other evens.
3 is as well to skip an otherwise necessary check in isPrime() */
int result = 5, count = 2;
for (int i = 5; count 1; }
if (/*(num & 1) == 0 || */ num % 3 == 0) { return false; }
int limit = (int)Math.sqrt(num);
for (int i = 5; i <= limit; i += 6) {
if (num % i == 0 || num % (i + 2) == 0) { return false; }
}
return true;
}
}I'm trying to adjust my programming style to a "people first" paradigm. While any and all feedback is welcome, that is the focus.
- Is there any ambiguity? Are the variable and method names adequate? I thought I could be more specific but the names would get significantly longer, is there some standard regarding the length of names?
- Are the comments sufficient? I structure a general purpose method, but comment out otherwise redundant checks; rendering its use distinct to the task. Given specificity it was also made private. Is this good practice?
- On the execution itself, is it satisfactory or are there some pitfalls I don't account for/optimizations I could make?
Solution
If you want a combination of readability and performance, consider the following solution.
If your concern is "people first", then I think this is an improvement. The
If your concern is performance, then the Sieve of Eratosthenes is a better algorithm for discovering many prime numbers. It doesn't matter how many ugly hacks you use to optimize your trial division method — the Sieve simply scales better (up to a point).
$$\begin{array}{r|r|r}
\textrm{First } n \textrm{ primes} & \textrm{Legato's trial division} & \textrm{200_success's sieve} \\
\hline
10^3 & ~1000\ \mathrm{\mu s} & ~2000\ \mathrm{\mu s} \\
10^4 & ~10\ \mathrm{ms} & ~10\ \mathrm{ms} \\
10^5 & ~125\ \mathrm{ms} & ~40\ \mathrm{ms} \\
10^6 & ~4000\ \mathrm{ms} & ~500\ \mathrm{ms} \\
10^7 & \color{gray}{\textit{DNF}} & ~7\ \mathrm{s} \\
\end{array}$$
In the interest of full disclosure…
public class Primes {
private final boolean[] isComposite;
private int n = 2;
public Primes(int sieveSize) {
this.isComposite = new boolean[sieveSize];
}
public static int overestimateOfNthPrime(int n) {
// http://en.wikipedia.org/wiki/Prime_number_theorem#Approximations_for_the_nth_prime_number
return (int)(1.5 * n * Math.log(n));
}
public int next() {
try {
return n;
} finally {
// Sieve of Eratosthenes: mark multiples of n as composite
for (int i = 2 * n; 0 0) {
sum += primes.next();
}
return sum;
}
public static void main(String[] args) {
long start = System.nanoTime();
System.out.println(sumFirstPrimes(Integer.parseInt(args[0])));
System.err.println(System.nanoTime() - start);
}
}If your concern is "people first", then I think this is an improvement. The
sumFirstPrimes() function is more readable because the code that discovers more primes is decoupled from the code that sums them. There are no magic numbers like 5 and 6, and no weird special cases. Note that the readability comes mainly from the code structure, not from the comments on the code.If your concern is performance, then the Sieve of Eratosthenes is a better algorithm for discovering many prime numbers. It doesn't matter how many ugly hacks you use to optimize your trial division method — the Sieve simply scales better (up to a point).
$$\begin{array}{r|r|r}
\textrm{First } n \textrm{ primes} & \textrm{Legato's trial division} & \textrm{200_success's sieve} \\
\hline
10^3 & ~1000\ \mathrm{\mu s} & ~2000\ \mathrm{\mu s} \\
10^4 & ~10\ \mathrm{ms} & ~10\ \mathrm{ms} \\
10^5 & ~125\ \mathrm{ms} & ~40\ \mathrm{ms} \\
10^6 & ~4000\ \mathrm{ms} & ~500\ \mathrm{ms} \\
10^7 & \color{gray}{\textit{DNF}} & ~7\ \mathrm{s} \\
\end{array}$$
In the interest of full disclosure…
- Making the estimate of the nth prime takes some mathematical knowledge.
- I trust the estimate, so I didn't bother to check the array bounds.
- For truly large problems (e.g 108), memory management for the Sieve would be a chore.
Code Snippets
public class Primes {
private final boolean[] isComposite;
private int n = 2;
public Primes(int sieveSize) {
this.isComposite = new boolean[sieveSize];
}
public static int overestimateOfNthPrime(int n) {
// http://en.wikipedia.org/wiki/Prime_number_theorem#Approximations_for_the_nth_prime_number
return (int)(1.5 * n * Math.log(n));
}
public int next() {
try {
return n;
} finally {
// Sieve of Eratosthenes: mark multiples of n as composite
for (int i = 2 * n; 0 <= i && i < this.isComposite.length; i += n) {
this.isComposite[i] = true;
}
do {
n++; // TODO: check array bounds
} while (this.isComposite[n]);
}
}
public static long sumFirstPrimes(int n) {
Primes primes = new Primes(overestimateOfNthPrime(n));
long sum = 0;
while (n-- > 0) {
sum += primes.next();
}
return sum;
}
public static void main(String[] args) {
long start = System.nanoTime();
System.out.println(sumFirstPrimes(Integer.parseInt(args[0])));
System.err.println(System.nanoTime() - start);
}
}Context
StackExchange Code Review Q#77559, answer score: 13
Revisions (0)
No revisions yet.