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

Test a number for primality and return smallest divisor different from 1 if it’s is composite

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

Problem

In this exercise we were asked to test a number for primality and print the results. Additionally – in the case the number is composite – the smallest divisor different from 1 should be printed out. Negative input values shall not pre processed and result in a message.

The second part of that task to improve the performance of the test and wipe out as many redundant and unnecessary comparisons as possible. We were not allowed to use any built-in methods to check for primality.

I have concerns mainly about properly handling input I’m rejecting and naming things (talking about result here).

```
/**
* Tests a positive integer for primality.
* Prints out whether a number is prime, composite or neither.
*/
public class PrimalityTest {
public static void main(String[] args) {
// We only test one number, so no other arguments are allowed.
if (args.length > 1) {
System.out.println("Too many arguments. Please enter a positive integer prime canidate.");
System.exit(1);
}

int primeCandidate = 0;
try {
primeCandidate = Integer.parseInt(args[0]);
} catch (NumberFormatException e) {
System.out.println("The entered value was not an integer.");
System.exit(1);
}

if (primeCandidate < 0) {
System.out.println("Negative numbers are never prime.");
System.exit(1);
}

int result = smallestDivisor(primeCandidate);
if (result == -1) {
System.out.println(primeCandidate + " is prime.");
} else if (result == 1 || result == 0) {
System.out.println(primeCandidate + " is neither prime nor composite.");
} else {
System.out.println(primeCandidate + " is composite and has the smallest divisor " + result + ".");
}
}

/**
* Returns the smallest divisor of composite numbers or
* 1 if the number is 1 (not composite, not prime) or
* -1 if

Solution

Single responsibility principle

You method smallestDivisor() violates the SRP because it does too many things.

It is

  • calculating the smallest divisor



  • calculating primes



  • checking if the number is neither composite nor prime



You should consider to just calculate what the name of the method tells you.

Method names should be made out of a verb or a verb phrase, so getSmallestDivisor() would be better.

So how to handle the edge cases ? That is easy, add methods to evaluate them.

private static boolean isPrime(int number, int smallestDivisor){
    return number == smallestDivisor;
}

private static boolean isComposite(int number, int smallestDivisor){
   return number != smallestDivisor && smallestDivisor > 1;
}


The getSmallestDivisor() method should then return as shown below

public static int smallestDivisor(int primeCandidate) {

    if (primeCandidate =< 1) {
        return primeCandidate;
    }
    if (primeCandidate % 2 == 0) {
        return 2;
    } 
    if (primeCandidate % 3 == 0) {
        return 3;
    } 

    // Checks divisability of numbers in the form of 6k+1 and 6k-1.
    // It starts with: 5 and 7, 11 and 13, 17 and 19, 23 and 25, ...
    for (int i = 5; i <= sqrt(primeCandidate); i += 6) {
        if (primeCandidate % i == 0) {
            return i;
        } else if (primeCandidate % (i + 2) == 0) {
            return i + 2;
        }
    }

    return primeCandidate;
}


to be tested like:

int smallestDivisor = getSmallestDivisor(primeCandidate);
    if (isPrime(primeCandidate, smallestDivisor)) {
        System.out.println(primeCandidate + " is prime.");
    } else if (isComposite(primeCandidate, smallestDivisor)) {
        System.out.println(primeCandidate + " is composite and has the smallest divisor " + smallestDivisor + ".");
    } else {
        System.out.println(primeCandidate + " is neither prime nor composite.");
    }


Input

  • you haven't checked an edge case: that a user might not pass any argument into the application.



  • you should extract the reading of the argument to a separate method and maybe add a possibility, if the given input doesn't match the requirements, to enter the value again. So e.g no arguments provided, value `

Code Snippets

private static boolean isPrime(int number, int smallestDivisor){
    return number == smallestDivisor;
}

private static boolean isComposite(int number, int smallestDivisor){
   return number != smallestDivisor && smallestDivisor > 1;
}
public static int smallestDivisor(int primeCandidate) {

    if (primeCandidate =< 1) {
        return primeCandidate;
    }
    if (primeCandidate % 2 == 0) {
        return 2;
    } 
    if (primeCandidate % 3 == 0) {
        return 3;
    } 

    // Checks divisability of numbers in the form of 6k+1 and 6k-1.
    // It starts with: 5 and 7, 11 and 13, 17 and 19, 23 and 25, ...
    for (int i = 5; i <= sqrt(primeCandidate); i += 6) {
        if (primeCandidate % i == 0) {
            return i;
        } else if (primeCandidate % (i + 2) == 0) {
            return i + 2;
        }
    }

    return primeCandidate;
}
int smallestDivisor = getSmallestDivisor(primeCandidate);
    if (isPrime(primeCandidate, smallestDivisor)) {
        System.out.println(primeCandidate + " is prime.");
    } else if (isComposite(primeCandidate, smallestDivisor)) {
        System.out.println(primeCandidate + " is composite and has the smallest divisor " + smallestDivisor + ".");
    } else {
        System.out.println(primeCandidate + " is neither prime nor composite.");
    }

Context

StackExchange Code Review Q#72113, answer score: 3

Revisions (0)

No revisions yet.