finding the prime numbers of an arraylist with toArray, prints wrong

Question

I have a method for finding the prime number from a list of numbers using ArrayList and toArray. The majority of the code works, except for the fact that when I'm printing the parameters 1 and 10, it prints 1,2,3,5,7. So it prints the prime numbers between 1 and 10, but it also prints 1. I think the error is in the first or second for-loop, but I'm not sure.

public int primtall(int a, int b) {
    ArrayList<Integer> primtallene = new ArrayList<>();
    int primtall = 0;
    int største;
    int minste;
    if(a == b){ 
        primtall = 0;
    }
    else {
        if(a > b) {
            største = a;
            minste = b;
        } 
        else if (a < b){
            minste = a;
            største = b;


        for(int i = minste; i <= største; i++) {
            boolean isPrime = true;
            for (int j = 2; j < i; j++) {
                if (i % j == 0) { 
                    isPrime = false;
                    break;
                }
            }
            if (isPrime) {

                primtall = i;
                primtallene.add(primtall);
            }
        }
    }

  }

  Integer[] numrene = new Integer[primtallene.size()];
  numrene = primtallene.toArray(numrene);
  for(Integer nummer : numrene){
      System.out.println("Primtall = " + nummer);
    }
    return 0;
}

Dont mind the last return value, I only added that because BlueJ requires me to.

Can anyone help with identifying the error in the code?

Thanks in advance!

I don't know if I disagree with any of the answers below, but the number 1 is neither prime nor composite. It should be subject to your prime test. — Tim Biegeleisen, Mar 05 '17 at 14:20

score 0 · Answer 1 · answered Mar 05 '17 at 14:11

for (int j = 2; j < i; j++) {
            if (i % j == 0) { 
                isPrime = false;
                break;
            }
        }

if i=1, (which would be the case when minste=1)the above loop will never be entered and isPrime = false; would never be executed. Hence, for i=1 is picked as prime.

score 0 · Answer 2 · answered Mar 05 '17 at 14:11

If you start with 1, minste will be 1 and the code never enters the second for loop, and isPrime ends up being true.

If you already know that 1 shouldn't be printed, you don't need to check it. Instead, you could start the first for loop with 2

score 0 · Accepted Answer · answered Mar 05 '17 at 14:14

That's becuase of the below for loop:

for (int j = 2; j < i; j++) {
    if (i % j == 0) { 
        isPrime = false;
        break;
    }
}

For the first time, i will be 1, which would make j < i condition return false and hence, control won't go into this for loop.

Due to this, isPrime will be true and 1 will get added into the list. To prevent this, you can just check whether i >= 2 in subsequent if condition, e.g.

if (isPrime && i >= 2) {
    primtall = i;
    primtallene.add(primtall);
}

Also, if you don't want to return any value from this method, you can change the return type as void and remove return statement.

finding the prime numbers of an arraylist with toArray, prints wrong

3 Answers3