-2
\$\begingroup\$

This method accepts an int parameter and verifies if the parameter is a prime number by trial division. It returns a Boolean value of true if it is prime or false otherwise.

public static boolean isPrime(int n) {
    int m = n / 2;
    for (int i = 2; i <= m; i++) {
        if (n % i == 0) {
            return false;
        }
    }
    return true;
}

I am particularly concerned with using modern coding practices.

\$\endgroup\$
  • \$\begingroup\$ Your code doesn't work. 0 and 1 are not prime. \$\endgroup\$ – Roland Illig Oct 27 '17 at 0:09
1
\$\begingroup\$

This is not really a question that ask about review. If you don't understand the stream then you are better to keep it like this.

But anyway..

1/ You iterate over all numbers between 2 and n/2 : IntStream.rangeClosed(2, n/2)

2/ You need to be sure that no numbers dividable by n : .noneMatch(i -> n % i == 0)

// Should be something like hat
return IntStream.rangeClosed(2, n/2)
            .noneMatch(i -> n % i == 0);
\$\endgroup\$
0
\$\begingroup\$

You are doing too much work here. Your limit m of n/2 is too large, sqrt(n) is a better limit. Since 2 is the only even prime you don't need to test any other even numbers as divisors. For completeness it is better to check for negative values of n up front. You might want to test your current code with negative numbers to see if it gives the correct answer.

For my taste, you use too many single letter variable names. Better to use more descriptive names: n -> num; m -> limit. I also prefer more commenting than you do. YMMV.

/** Returns true if num is prime, false otherwise. */
public static boolean isPrime(int num) {
    // Low numbers.
    if (num <= 1) {
        return false;  // negatives, 0, 1 not prime.
    }
    // Even numbers.
    if (num % 2 == 0) {
        return num == 2;  // 2 is the only even prime.
    }
    // Odd numbers >= 3
    int limit = (int)Math.sqrt(num);
    // Start loop at 3 and step 2 so we only try odd numbers as divisors.
    for (int i = 3; i <= limit; i += 2) {
        if (num % i == 0) {
            return false;
        }
    }
    return true;
} // end isPrime()
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.