patternjavaMinor
Test a number for primality and return smallest divisor different from 1 if it’s is composite
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
```
/**
* 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
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
It is
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
So how to handle the edge cases ? That is easy, add methods to evaluate them.
The
to be tested like:
Input
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 belowpublic 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.