I am currently reading and working through "Clean Code: A Handbook of Agile Software Craftsmanship" by Robert C. Martin. The author talks about how a function should do one thing only, and thus be relatively short. Specifically Martin writes:
This implies that the blocks within if statements, else statements, while statements, and so on should be one line long. Probably that line should be a function call. Not only does this keep the enclosing function small, but it also adds documentary value because the function called within the block can have a nicely descriptive name.
This also implies that functions should not be large enough to hold nested structures. Therefore, the indent level of a function should not be greater than one or two. This, of course, makes the functions easier to read and understand
This makes sense, but seems to conflict with examples of what I see as clean code. Take the following method for example:
public static boolean millerRabinPrimeTest(final int n) {
final int nMinus1 = n - 1;
final int s = Integer.numberOfTrailingZeros(nMinus1);
final int r = nMinus1 >> s;
//r must be odd, it is not checked here
int t = 1;
if (n >= 2047) {
t = 2;
}
if (n >= 1373653) {
t = 3;
}
if (n >= 25326001) {
t = 4;
} // works up to 3.2 billion, int range stops at 2.7 so we are safe :-)
BigInteger br = BigInteger.valueOf(r);
BigInteger bn = BigInteger.valueOf(n);
for (int i = 0; i < t; i++) {
BigInteger a = BigInteger.valueOf(SmallPrimes.PRIMES[i]);
BigInteger bPow = a.modPow(br, bn);
int y = bPow.intValue();
if ((1 != y) && (y != nMinus1)) {
int j = 1;
while ((j <= s - 1) && (nMinus1 != y)) {
long square = ((long) y) * y;
y = (int) (square % n);
if (1 == y) {
return false;
} // definitely composite
j++;
}
if (nMinus1 != y) {
return false;
} // definitely composite
}
}
return true; // definitely prime
}
}
This code is taken from the Apache Commons source code repo at: https://github.com/apache/commons-math/blob/master/src/main/java/org/apache/commons/math4/primes/SmallPrimes.java
The method looks very readable to me. For algorithm implementations like this one (implementation of Miller-Rabin Probabilistic Primality Test), is it suitable to keep the code as is and still consider it 'clean', as defined in the book? Or would even something already as readable as this benefit from extracting methods to make the algorithm essentially a series calls to functions that "do one thing only"? One quick example of a method extraction might be moving the first three if-statements to a function like:
private static int getTValue(int n)
{
int t = 1;
if (n >= 2047) {
t = 2;
}
if (n >= 1373653) {
t = 3;
}
if (n >= 25326001) {
t = 4;
}
return t;
}
Note: This question is different that the possible duplicate (though that question is helpful to me too), because I am trying to determine if I am understanding the intention of the author of Clean Code and I am providing a specific example to make things more concrete.