HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Bulls and Cows AI

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
andbullscows

Problem

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:

  • BandC - where all the logic is.



  • 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 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 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 generatePossibleNums() {
Set set = new HashSet();
for (int i = 1000; i iter = set.iterator();
while (iter.hasNext()) {
String str = iter.next();
Set digits = new HashSet<>();
for (char c : str.toCharArray()) {
if (digits.contain

Solution

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 possibilities = new HashSet();

    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 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 digits) {
if (possibility.length() >= MAXIMUM_SIZE) {
possibilities.add(possibility.toString());
return;
}

for (int i = 0; i digits = new LinkedList<>(Arrays.asList(0, 1, 2, 3, 4, 5

Code Snippets

if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
if (obj == null || getClass() != obj.getClass()) {
            return false;
        }
if (bullCount != other.bullCount)
            return false;
        if (cowCount != other.cowCount)
            return false;
        return true;
return bullCount == other.bullCount && cowCount == other.cowCount;
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;
            }
        }
    }

Context

StackExchange Code Review Q#156647, answer score: 2

Revisions (0)

No revisions yet.