1

I was implementing Sieve's Algorithm for finding Prime Numbers up to n. I am unable to find out why is it running into infinite loop.

Here I'm giving the code snippet. Please help.

for(int j=2;j<=Math.sqrt(n);j++){
  if(a[j]==true){
     int x=0;
     for(int p=(j*j+x*j); p<=n;x++){
        a[p]=true;
     }
  }
}
tourist
  • 506
  • 1
  • 6
  • 20
  • 1
    When you correct it, keep in mind that an addition is cheaper than a multiplication and write the inner cycle as `for(int p=j*j; p<=n;p+=j)`. – Adrian Colomitchi Dec 06 '16 at 10:15
  • Also, shouldn't the sieve take the first number not marked as composite and mark all its multiples as composite? – biziclop Dec 06 '16 at 10:24
  • @biziclop "shouldn't the sieve take the first number not marked as composite" where do you think this doesn't happen? (the first `p` that is non-composite is indeed `j*j` - any lower than that would have been marked as composite when `j` have had lower values) – Adrian Colomitchi Dec 06 '16 at 10:28
  • I'm very stupid this morning but isn't `j*j` by definition composite for any `j>1`? :) – biziclop Dec 06 '16 at 10:44
  • You may want to update your title to "Sieve of Eratosthenes" so more people realize what this question is about. This algorithm is not called "Sieve's Algorithm". – Andrew Dec 06 '16 at 13:08

2 Answers2

5

Your inner loop is checking p but never changing it

Caleth
  • 52,200
  • 2
  • 44
  • 75
1

Some optimisations suggestions:

// When j is 2, the bitwise (2 & 1) will be 0, so the cycle increments by 1
// When j is odd (from 3 and above), the cycle goes in steps of 2 
// (thus exploring only odd numbers)
// As a result, this cycle is twice as faster than the one progressing
// in increments of 1 
// See here --------------------V
for(int j=2;j<=Math.sqrt(n);j+=(j & 1 ? 2 : 1)) {
  if(a[j]==true){
     // int x=0; // what for?
     // a sum is cheaper than a multiplication
     // Here --V and here --V
     for(int p=j*j; p<=n;p+=j){
        a[p]=true;
     }
  }
}
Adrian Colomitchi
  • 3,974
  • 1
  • 14
  • 23