Cleaner Code

My team of software developers at work has decided (with some consultation by our team leader) to have a biweekly gathering to discuss a chapter of “Clean Code”. I am on vacation just now and had to miss the first meeting, but I am just reading the book on the train home and here's a little insight I want to share. I am talking about the last example of Chapter 2 in the section "Add meaningful context".

I think that the general strategy of giving a bunch of variables a context by putting them in a separate class is good, so I don't object with the point of the book.
However, I also think that this particular example can be improved in another way, which gets rid of the variables altogether by making the code simpler and shorter.

First of all, the naming of the method is wrong. Most of it is concerned with formatting the GuessStatistics, so I'd rename it "formatGuessStatistics" and refactor the call to print out to the calling method. This will also rid us of the dependency to however the statistics are printed.

Now, let's recognize that the method actually does two things: first, recognize the plural which is applied to all numbers but "1" and results in a different verb and plural "s", and second, replace the number "0" with the word "no". Instead of flattening those two choices into three cases, we should seperate the concerns.

private String formatGuessStats(char candidate, int count) {
    final String number = count==0 ? "no" : Integer.toString(count);
    if (count == 1) {
        return String.format("There is 1 %s", candidate);
    } else {
        return String.format("There are %s %ss", number, candidate);
    }
}

Maybe you'll think that I introduced bad redundancy by repeating the word "There ". I, however, think that such a little bit of redundancy is of no harm, especially since in this case it helps us remove abstraction and see more directly what the code is doing. I also think that the redundancy is only accidental a mirrors redundancy in the English language to which we convert here. If, for example, our PO decides that the singular case should read "There's" instead of "There is", our simplified (yet redundant) variant will be a bit easier to change.

Now, let's look at some further minor improvements of this code. Observing that the "number" variable is only used in the second part, we can move it down into the else block.

private String formatGuessStats(char candidate, int count) {
    if (count == 1) {
        return String.format("There is 1 %s", candidate);
    } else { 
        final String number = count==0 ? "no" : Integer.toString(count);
        return String.format("There are %s %ss", number, candidate);
    }
}

Also we could simplify some more and use the handy "%d" instead of the wordy "Integer.toString". If you are tempted to add a comment to the else-block saying something like "// handle plural case", you can as well factor it out to a second method.

private String formatGuessStats(char candidate, int count) {
    if (count == 1) {
        return String.format("There is 1 %s", candidate);
    } else { 
        return formatPluralGuessStats(candidate, count); 
    }
}

private String formatPluralGuessStats(char candidate, int count)  {
    if (count == 0) {
        return String.format("There are no %ss", candidate);
    } else {
        return String.format("There are %d %ss", count, candidate);
    }
}

Incidentally, this leaves us with code that doesn't contain any local variables any more at all. Given that it is so simple now, we could go back to using just one method and sort the cases in increasing order of "count":

private String formatGuessStats(char candidate, int count) {
    if (count == 0) {
        return String.format("There are no%ss", candidate);
    } else if (count == 1) {
        return String.format("There is 1 %s", candidate);
    } else { 
        return String.format("There are %d %ss", count, candidate);
    }
}

Admittedly we now have reintroduced the three cases from the original code. But isn't it so much more direct and clear?

Which variant do you prefer? The original, the final, or any of the intermediate ones?

PS: When continuing to read the book, I found that some of the principles I used in doing this refactoring are also introduced in the book. Apparently not all of the examples used comply with all the rules given. In particular I got very upset about the use of a parameter for output in a later example and went on to write a long rant about why this is bad and how it can be avoided. Two chapters later, Uncle Bob himself states that this is bad and gave the same alternative techniques on how to avoid the problem. I guess this means that at least Uncle Bob agrees with my own principles of coding... PPS: Bloggers new composition interface almost doesn't suck anymore. Good job, guys! Keep it up!