0

I wrote this code which is supposed to search for prime numbers and put them into an array. Here it is:

int[] prime_array = new int[(int)s.upper_bound];
int index_in_array = 0;
boolean are_we_done = false;
int index = 1;
boolean is_prime = true;

while (!are_we_done) {
    try {
        for (int i = 1; i < index; i++) {
            if ((index%i)==0) {
                is_prime = false;
            }
        }

        if (is_prime) {
            prime_array[index_in_array] = index;
            index_in_array++;
        }

        index++;
        is_prime = true;
    } 
    catch (IndexOutOfBoundsException e) {
        are_we_done = true;
        break;
    }
}

Unfortunately, the only prime it catches is 1. Any idea why it doesn't work?

takendarkk
  • 2,949
  • 7
  • 22
  • 35
SalmonKiller
  • 2,123
  • 3
  • 21
  • 55
  • Your title suggests the *algorithm* is no good, but I think you mean your own implementation by that. – Jongware Nov 29 '14 at 23:43
  • @Dici Why not? I have heard about for loops, but in my opinion reading this code is easier. – SalmonKiller Nov 29 '14 at 23:48
  • @Dici why? In this case, the only way the `ArrayOutOfBoundException` can be thrown is when the array is full. – SalmonKiller Nov 29 '14 at 23:50
  • You can very easily check the moment when you have filled your array, so why would you wait an exception to be thrown and catch it ? You canno rely on a programming error to exit a loop !! Plus, exceptions are slow, slow, slow. – Dici Nov 29 '14 at 23:52
  • [Why not use exceptions as regular flow of control?](http://stackoverflow.com/questions/729379/why-not-use-exceptions-as-regular-flow-of-control) – zapl Nov 29 '14 at 23:52
  • @Dici Now that's something I will accept and fix. Thank you. – SalmonKiller Nov 29 '14 at 23:53
  • Looking at this your first loop does not run when index=1 as i is 1 and is recorded as a success, otherwise every number is divisible by 1 including primes. – Matthew V Carey Nov 29 '14 at 23:59

1 Answers1

5

Your for loop is looking for factors starts with i=1, and if index%i==0 you decide it is not a prime. But n%1==0 for all integers n.

The lowest factor that indicates that something is not prime is 2.

Edit:

Here are some other suggestions:

  • Use a properly bounded loop to cycle through your candidate primes.
  • Only check factors up to the square root of the candidate prime.
  • Only check 2 and odd numbers as primes (because no even number above 2 is prime, and 1 is a unit, not a prime)
  • Only check 2 and odd numbers as factors (because no even number above 2 can be a prime factor)
  • Use a sieve instead.
khelwood
  • 46,621
  • 12
  • 59
  • 83
  • That is not the only problem. This does not address the question – Dici Nov 29 '14 at 23:47
  • @Dici I certainly agree that the code is bad in lots of other ways, but this is the problem the OP was asking about. – khelwood Nov 29 '14 at 23:48
  • @khelwood what other ways? – SalmonKiller Nov 29 '14 at 23:49
  • @khelwood The answer is now complete, +1. However, it returns 1 as a prime, to fix it, initialize `is_prime` to false. – Dici Nov 29 '14 at 23:57
  • @khelwood The part about only the square root is right. However, I can't use a for loop since I don't know how many primes I will need. Same reason for why it is not as efficient to use a sieve. – SalmonKiller Nov 30 '14 at 00:04