Clean Code, part 3

In this multipart series, I take some perfectly working code which is definitely not clean and refactor it into a much cleaner and more readable implementation. If you disagree with my approach at any point then I ask that you voice your concerns in the comment section. In part 1 of this series, I introduced what I see as the problem and the code that I will be refactoring in subsequent posts. In part 2 of the series, I made the code a little more readable by refactoring it a little. The code was still not object-oriented, however, so here is part 3, in which we split the functionality up into a few different classes.

One major issue with the existing code is that the target solution is passed along at every stage. This means that on every turn of the game, the application must pass in the target as well as the guess. It would be much better if we could just pass that value once. We'll do that by creating a HangmanSolution class. This class will keep track of the target value as well as the partial solution.

[pre class="brush:java" title="HangmanSolution.java"]package com.bitsnibblesandbytes.hangman;

class HangmanSolution {
 
    private String target;
 
    private String partialSolution;
 
    HangmanSolution(String target) {
        target = this.target;
        createPartialSolution();
    }
 
    String getTarget() {
        return target;
    }
 
    String getPartialSolution() {
        return partialSolution;
    }
 
    int checkCharacter(char guess) {
        int count = 0;
        for(int i = 0; i < target.length(); i++) {
            if (updatePartialSolution(guess)) {
                count++;
            }
        }
        return count;
    }
 
    boolean isSolutionFound() {
        return target.equalsIgnoreCase(partialSolution);
    }
 
    private boolean updatePartialSolution(char guess) {
        boolean guessMatches = target.toLowerCase().charAt(i) == Character.toLowerCase(guess);
        if (guessMatches) {
            partialSolution = partialSolution.substring(0, i) + Character.toLowerCase(guess) + partialSolution.substring(i + 1);
        }
        return guessMatches;
    }

    private void createPartialSolution() {
        StringBuilder builder = new StringBuilder();
        for (int i = 0; i < target.length(); i++) {
            builder.append('*');
        }
        partialSolution = builder.toString();
    }
}
[/pre]

Most of the above logic is simply copied from the GamePlayer class we created in part 2. The one exception is the isSolutionFound() method. We will see where this method gets used a little later in this post. Note that I did not make this class or any of its methods public. This is because this class is simply meant to be used within this package, so package-private works. There's no point in adding something to my public API that I don't intend the public to use.

Another problem we have is that the GamePlayer class does everything. It should really just direct traffic as other classes do all of the heavy lifting. The HangmanSolution class starts us down the road to removing functionality. Let's move it a little further by pulling the guess validation code out into its own class:

[pre class="brush:java" title="GuessValidator.java"]package com.bitsnibblesandbytes.hangman;

class GuessValidator {
 
    private String previousGuesses = "";
 
    boolean isLetter(char guess) {
        return Character.isAlphabetic(guess);
    }
 
    boolean isAvailableLetter(char guess) {
        if (guesses.indexOf(Character.toLowerCase(guess)) < 0) {
            guesses += Character.toLowerCase(guess);
            return true;
        }
        return false;
    }
}
[/pre]

Next, we'll pull out the logic for building the response string:

[pre class="brush:java" title="ResponseBuilder.java"]package com.bitsnibblesandbytes.hangman;

class ResponseBuilder {
 
    static buildResponse(int count, char guess, HangmanSolution solution, boolean gameLost) {
        String feedbackMessage = buildFeedbackMessage(guess, count);
        String winLoseOrContinueMessage = buildWinLoseOrContinueMessage(solution, gameLost);
        return feedbackMessage + "\n" + winLoseOrContinueMessage
    }

    private static String buildWinLoseOrContinueMessage(HangmanSolution solution, boolean gameLost) {
        String winLoseOrContinue = "";
        if (gameLost) {
            winLoseOrContinue += "The word is " + solution.getTarget() + "\nYou lose.";
        } else if (solution.isSolutionFound()) {
            winLoseOrContinue += "The word is " + solution.getPartialSolution() + "\nYou win!";
        } else {
            winLoseOrContinue += solution.getPartialSolution();
        }
        return winLoseOrContinue;
    }

    private static String buildFeedbackMessage(char guess, int count) {
        String verb;
        String pluralModifier;
        String number;
        if (count == 0) {
            number = "no";
            verb = "are";
            pluralModifier = "s";
        } else if (count == 1) {
            number = "1";
            verb = "is";
            pluralModifier = "";
        } else {
            number = Integer.toString(count);
            verb = "are";
            pluralModifier = "s";
        }
        return String.format("There %s %s %s%s", verb, number, guess, pluralModifier);
    }
}
[/pre]

Note that the methods here are static since the ResponseBuilder does not depend on its own state at all. Some would find this a code smell, and I agree that this isn't the prettiest class, but at least it gets all of that string building out of the GamePlayer class, paring it down to a pretty short class:

[pre class="brush:java" title="GamePlayer.java"]package com.bitsnibblesandbytes.hangman;

public class GamePlayer {
 
    private static final int MAX_ERRORS = 5;

    private int errors = 0;

    private HangmanSolution solution;
 
    private GuessValidator validator;
 
    public GamePlayer(String solution) {
        this.solution = new HangmanSolution(solution);
        validator = new GuessValidator();
    }

    public String handleGuess(char guess) {
        if (!validator.isLetter(guess)) {
            return "Error: you can only guess letters.";
        }
        if (!validator.isAvailableLetter(guess)) {
            return "You already guessed that";
        }
        int count = solution.checkCharacter(guess);
        if (count == 0) {
            errors++;
        }
        return ResponseBuilder.buildResponse(count, guess, solution, errors > MAX_ERRORS);
    }
}
[/pre]

This concludes the refactoring I'm going to do. It's not the most elegant solution, but without starting over from scratch we have at least achieved cleaner code than we started with.

In part 4 of the series, we'll add a simple feature quickly and easily that would not have been so easy to implement with the code we started with in part 1.