HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaModerate

Sum of First N Primes

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
firstprimessum

Problem

Task: Print sum of first 1000 primes.

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.

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.