What Clean Code Looks Like in Real Life – Practical Examples

clean code

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.

Best Answer

"Clean code" is not an end in itself, it is a means to an end. The main purpose of refactoring bigger functions into smaller ones and cleaning up the code in other ways is to keep the code evolvable and maintainable.

When picking such a very specific mathematical algorithm like the "Miller-Rabin" prime test from a text book, most programmers do not want to evolve it. Their standard goal is to transfer it from the pseudo code of the text book correctly into the programming language of their environment. For this purpose, I would recommend to follow the text book as close as possible, which typically means not to refactor.

However, for someone working as mathematician in that field who is trying to work on that algorithm and change or improve it, IMHO splitting up this function into smaller, well named ones, or replacing the bunch of "magic numbers" by named constants, can help to make changes to the code easier like for any other kind of code as well.

Related Topic