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

Find prime numbers under N

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

Problem

I'm using the Sieve of Eratosthenes to find primes numbers under N. Is there any performance problems you see with my approach? I used the LinkedList because of the performance of removing items to the list while looping through it. Also, I'd want to know if I respect the Java conventions with my code.

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

public class PrimeFinders
{
    public static void main (String[] args) throws java.lang.Exception
    {
        LinkedList naturalNumbers = new LinkedList();
        final int maxNumber = 1000; //This is N

        //Add only uneven numbers, as even numbers won't be prime
        for(int i = 3; i  0){
            primeNumber = naturalNumbers.pop(); //Always a prime since non-prime numbers are removed in the loop.
            System.out.println("Prime " + primeIndex + " : " + primeNumber);
            applyEratosthenesSieve(primeNumber, naturalNumbers);
            primeIndex++;
        }
    }

    private static void applyEratosthenesSieve(int prime, LinkedList numbers){
        Iterator numbersIterator = numbers.iterator();

        while(numbersIterator.hasNext()){
            int number = numbersIterator.next();
            if(number % prime == 0){ //Remove all multiples of the number.
                numbersIterator.remove();
            }
        }
    }
}

Solution

This doesn't look like a Sieve of Eratosthenes. This sure looks like trial division:

while(numbersIterator.hasNext()){
      int number = numbersIterator.next();
      if(number % prime == 0){ //Remove all multiples of the number.
          numbersIterator.remove();
      }
  }


Unless I'm misunderstanding what your iterators are doing. A simple SoE will have something that looks like:

for i from 2 to sqrt(n):          /* Loop from 2 to square root of max */
  if A[i]:                        /* If this location indicates prime, */
    for j from i*i to n step i:   /* then for each multiple of i,      */
      A[j] = false;               /* mark composite                    */


See the examples at RosettaCode or Wikipedia for example. For comparison, RosettaCode's Trial Division task has a loop like your function.

So the immediate performance issue is that a sieve will be much faster than trial division.

Code Snippets

while(numbersIterator.hasNext()){
      int number = numbersIterator.next();
      if(number % prime == 0){ //Remove all multiples of the number.
          numbersIterator.remove();
      }
  }
for i from 2 to sqrt(n):          /* Loop from 2 to square root of max */
  if A[i]:                        /* If this location indicates prime, */
    for j from i*i to n step i:   /* then for each multiple of i,      */
      A[j] = false;               /* mark composite                    */

Context

StackExchange Code Review Q#62361, answer score: 8

Revisions (0)

No revisions yet.