Bulls and Cows AI

Question

This is a program with an 'AI' that plays Bulls and cows.

Bulls and cows is a code-breaking game where you think of 4 digit number and the other layer tries to guess it: https://en.wikipedia.org/wiki/Bulls_and_Cows

There are two classes:

1. BandC - where all the logic is.
2. BcCount - which is a pair of bull and cow count.

Any advice is welcome. Especially overall design/making my code more idiomatic and readable/simple.

package BullsAndCows;

import java.util.HashSet;
import java.util.Iterator;
import java.util.Random;
import java.util.Scanner;
import java.util.Set;

public class BandC {
    public static void main(String[] args) {
        Set<String> possibleNums = generatePossibleNums();
        int steps = 0;
        System.out.println("Hello! I am your computer!\nNow think of a number...");
        Scanner reader = new Scanner(System.in);
        while (true) {
            steps++;
            Iterator<String> iter = possibleNums.iterator();
            String AIguess = iter.next();
            System.out.println("My guess is: " + AIguess);
            System.out.print("Number of cows:");
            int numberOfCows = reader.nextInt();
            System.out.print("Number of bulls:");
            int numberOfBulls = reader.nextInt();
            removeWrongNums(new BcCount(numberOfBulls, numberOfCows), AIguess, possibleNums);
            if (numberOfBulls == 4) {
                System.out.println("Guessed in " + steps + " steps");
                break;
            }
        }
        reader.close();
    }

    public static BcCount calcBullandCowCount(String guess, String candidate) {
        BcCount bcCount = new BcCount(0, 0);
        for (int i = 0; i < candidate.length(); i++) {
            if (guess.charAt(i) == candidate.charAt(i)) {
                bcCount.bullCount++;
            } else if (guess.contains(String.valueOf(candidate.charAt(i)))) {
                bcCount.cowCount++;
            }
        }
        return bcCount;
    }

    public static Set<String> generatePossibleNums() {
        Set<String> set = new HashSet<String>();
        for (int i = 1000; i < 10_000; i++) {
            set.add(String.valueOf(i));
        }
        Iterator<String> iter = set.iterator();
        while (iter.hasNext()) {
            String str = iter.next();
            Set<Character> digits = new HashSet<>();
            for (char c : str.toCharArray()) {
                if (digits.contains(c)) {
                    iter.remove();
                    break;
                }
                digits.add(c);
            }
        }
        return set;
    }

    public static void removeWrongNums(BcCount guessBcCount, String guess,
            Set<String> possibleNums) {
        Iterator<String> iter = possibleNums.iterator();
        while (iter.hasNext()) {
            String str = iter.next();
            if (calcBullandCowCount(guess, str).equals(guessBcCount) == false) {
                iter.remove();
            }
        }
    }

}

BcCount.java

package BullsAndCows;

public class BcCount {
    public int bullCount = 0;
    public int cowCount = 0;

    public BcCount(int b, int c) {
        bullCount = b;
        cowCount = c;
    }

    public String toString() {
        return bullCount + " " + cowCount;
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        BcCount other = (BcCount) obj;
        if (bullCount != other.bullCount)
            return false;
        if (cowCount != other.cowCount)
            return false;
        return true;
    }
}

Answer 1

Boolean operators are fun

        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;

You can write this more briefly as

        if (obj == null || getClass() != obj.getClass()) {
            return false;
        }

The exact same effect (short circuiting means that only the first operand will be evaluated if true) and less confusion about what's being returned. We have a true exit and a false exit.

I also prefer to always use the block form of control structures for consistency, readability, and robustness. I don't want to relitigate it if you know all this already, but I did want to mention it for those who do not.

Return Boolean results directly

        if (bullCount != other.bullCount)
            return false;
        if (cowCount != other.cowCount)
            return false;
        return true;

This can be written more briefly as

        return bullCount == other.bullCount && cowCount == other.cowCount;

Not only is this shorter, but I find it more readable. If both fields are equal, then we consider the objects equal.

Using the class

The existing code puts everything in main and uses static methods. Consider instead

    private final static int MAXIMUM_SIZE = 4;

    private int steps = 0;
    private Set<String> possibilities = new HashSet<String>();

    public static void main(String[] args) {
        BandC game = new BandC();
        game.generatePossibilities();
        System.out.println("Hello! I am your computer!\nNow think of a number...");
        try (Scanner reader = new Scanner(System.in)) {
            game.play(reader);
        }
    }

    public void play(Scanner reader) {
        while (true) {
            steps++;

            Iterator<String> iter = possibilities.iterator();
            String AIguess = iter.next();
            System.out.println("My guess is: " + AIguess);

            System.out.print("Number of cows:");
            int numberOfCows = reader.nextInt();
            System.out.print("Number of bulls:");
            int numberOfBulls = reader.nextInt();

            removeWrongNums(new BcCount(numberOfBulls, numberOfCows), AIguess);

            if (numberOfBulls == 4) {
                System.out.println("Guessed in " + steps + " steps");
                break;
            }
        }
    }

This moves the game into separate methods that could be called by a different program.

I added some vertical whitespace to separate the code into paragraphs of common functionality. I find this easier to read.

This also uses the try-with-resources form with the Scanner. This way it still closes automatically if there is an Exception. And you don't have to explicitly tell it to close regardless.

I would consider just telling the program the number and letting it calculate the cows and bulls. Don't worry, it won't tell itself. Computers are good at paying attention to detail.

It seems like there should be some better AI approaches, but this guesses pretty quickly. Perhaps I'm just bad at generating sequences.

When there's only one possibility left, it still asks for a review of its guess. It seems like it should tell you that it's the only possibility. Of course, that would look less like the original game.

Don't do unnecessary work

        for (int i = 1000; i < 10_000; i++) {

Since you remove sequences that have repeat digits, you could just as well say

        for (int i = 1023; i <= 9876; i++) {

All the numbers in the original range less than 1023 contain duplicate digits and all the ones greater than 9876 do. So why put them into the set?

An alternative

It's quite possible to generate just the valid possibilities. You don't have to remove the invalid ones.

    public boolean isValid(String possibility) {
        for (int i = 0; i < possibility.length(); i++) {
            if (possibility.substring(i + 1).contains(possibility.substring(i, i + 1))) {
                return false;
            }
        }

        return true;
    }

    public void generatePossibilities() {
        for (int i = 1023; i <= 9876; i++) {
            String possibility = String.valueOf(i);
            if (isValid(possibility)) {
                possibilities.add(possibility);
            }
        }
    }

Moving the validation into its own method makes it easier to check. Instead of a nested continue, we can do a simple if.

I prefer generatePossibilities to generatePossibleNums.

Another alternative

It's also possible to do this at the digit level, but it's fiddly enough that I'm not sure that it's faster. A bit easier to expand to an arbitrary number of digits.

    private void generateNextDigit(StringBuilder possibility, List<Integer> digits) {
        if (possibility.length() >= MAXIMUM_SIZE) {
            possibilities.add(possibility.toString());
            return;
        }

        for (int i = 0; i < digits.size(); i++) {
            int digit = digits.remove(i);
            possibility.append(digit);
            generateNextDigit(possibility, digits);
            digits.add(i, digit);
            possibility.deleteCharAt(possibility.length() - 1);
        }
    }

    public void generatePossibilities() {
        List<Integer> digits = new LinkedList<>(Arrays.asList(0, 1, 2, 3, 4, 5, 6, 7, 8, 9));
        StringBuilder possibility = new StringBuilder(MAXIMUM_SIZE);
        for (int i = 1; i < digits.size(); i++) {
            int digit = digits.remove(i);
            possibility.append(digit);
            generateNextDigit(possibility, digits);
            digits.add(i, digit);
            possibility.deleteCharAt(0);
        }
    }