Evaluating result for mastermind comparison

Question

I'm implementing a Java version of the game Mastermind.

My version uses numbers to represent colors, for example 2315. This function compares a string of numbers to the secret code the object stored. It works, but I'm wondering if this could be done in a more elegant way.

public Answer evaluate(String guess) {
    boolean[] codeUsed = new boolean[code.length()];
    boolean[] guessUsed = new boolean[guess.length()];

    int correct = 0;
    int match = 0;

    // Compare correct color and position
    for (int i = 0; i < code.length(); i++) {
        if (code.charAt(i) == guess.charAt(i)) {
            correct++;
            codeUsed[i] = guessUsed[i] = true;
        }
    }

    // Compare matching colors for "pins" that were not used
    for (int i = 0; i < code.length(); i++) {
        for (int j = 0; j < guess.length(); j++) {
            if (!codeUsed[i] && !guessUsed[j] && code.charAt(i) == guess.charAt(j)) {
                match++;
                codeUsed[i] = guessUsed[j] = true;
                break;
            }
        }
    }

    return new Answer(guess, correct, match, guess.length() - correct - match);
}

Answer 1

The algorithm is short and concise. There might be a more clever way to do it, but it would likely be less readable.

This code will break if the guess is not the same length as the code. An empty string causes an out of out of bounds exception. A string longer than the code could cause other problems. You don't show what Answer is, but it looks like the last argument is the number of incorrect pins. This could lead to having more incorrect pins than there are in the code. You need to protect against each of these cases.

Since you are using strings, the value could contain characters other than digits. A guess with invalid characters should be treated differently than an incorrect guess.

You can add a check before the second loop to see if the all of the characters are correct. In that case you would skip over a N x M loop.

Answer 2

There's an alternative and slightly simpler algorithm that works by counting the number of times each color appears in the code and in the answer. The total number of pins (white and black) in the evaluation is then given by the sum (over all colors) of whichever is the smaller of these. Here's how you might implement it (I've written COLORS for the number of different colors):

public Answer evaluate(String guess) {
    // Total number of pins in the answer.
    int pins = 0;

    // Number of black pins in the answer.
    int black = 0;

    // Number of times each color appears in the code and the guess.
    int[] codeColorCount = new int[COLORS];
    int[] guessColorCount = new int[COLORS];

    for (int i = 0; i < code.length(); i++) {
        char c = code.charAt(i);
        char g = guess.charAt(i);
        ++ codeColorCount[c.getNumericValue()];
        ++ guessColorCount[g.getNumericValue()];
        if (c == g) {
            ++ black;
        }
    }

    for (int i = 0; i < COLORS; i++) {
        pins += Math.min(codeColorCount[i], guessColorCount[i]);
    }

    return new Answer(guess, black, pins - black, guess.length() - pins);
}

(Since the numbers in codeColorCount never change during a game, you might decide to cache them.)

In some languages this algorithm can be expressed tersely, for example in Python you could write it like this:

from collections import Counter
def evaluate(code, guess):
    pins = sum((Counter(code) & Counter(guess)).values())
    black = sum(c == g for c, g in zip(code, guess))
    return guess, black, pins - black, len(guess) - pins

Answer 3

boolean[] codeUsed = new boolean[code.length()];
boolean[] guessUsed = new boolean[guess.length()];
codeUsed[i] = guessUsed[i] = true;
if (!codeUsed[i] && !guessUsed[j] && ...) {
codeUsed[i] = guessUsed[j] = true;

I can not see why both of them are used? You always set both of them to the same value and access both of them. So one can safely be removed.

return new Answer(guess, correct, match, guess.length() - correct - match);

In general, you should try to reduce the number of arguments. In this case, you do not need the fourth argument, because you can easily calculate it inside the Answer object.

for (int i = 0; i < code.length(); i++) {
    for (int j = 0; j < guess.length(); j++) {
        if (!codeUsed[i] && !guessUsed[j] && code.charAt(i) == guess.charAt(j)) {
            match++;
            codeUsed[i] = guessUsed[j] = true;
            break;
        }
    }
}

You do not need to keep track of the already used parts. Just do a match = correct - matchor something like this after you have counted all matches. To simplify this, i would count the appearance in a temporary string, which is reduced for every hit.

I would add some error checking for the guess argument. Could be helpful if something weird happens.

All combined, it could look like this:

public Answer evaluate(final String guess) {
    if (guess == null || guess.isEmpty() || guess.length() != code.length())
        throw new IllegalArgumentException("Invalid input. guess: " + guess + ", code: " + code);

    int correct = 0;
    int matchAll = 0;

    String reducedString = guess;
    for (int i = 0; i < code.length(); i++) {
        if (code.charAt(i) == guess.charAt(i))
            ++correct;

        final String currentCharAsString = String.valueOf(code.charAt(i));
        if (reducedString.contains(currentCharAsString))
        {
            ++matchAll;
            reducedString = reducedString.replaceFirst(Pattern.quote(currentCharAsString), "");
        }
    }
    return new Answer(guess, correct, matchAll - correct);
}

After all, you should write some unit tests to check the code.