Master Mind implementation

Question

I have created a Master Mind BoardSolver. To apply the BoardSolver I have created an app (MasterMind) in which I create a Board with ColorPins and CheckPins. ColorPins and CheckPins are associated within a PinRow. A Board represents a given PinRow (code) and a List of PinRows as attempts.

Please can you review this code?

Structure

Code

Board

package com.github.martinfrank.mastermind;

import java.util.ArrayList;
import java.util.List;

public class Board {

    private final int maxAttempts;
    private final PinRow code;
    private final List<PinRow> attempts = new ArrayList<>();

    public Board(PinRow code, int maxAmountTrials) {
        this.code = code;
        this.maxAttempts = maxAmountTrials;
    }

    public List<PinRow> getAttempts() {
        return attempts;
    }


    public List<CheckPin> addAttempt(PinRow trial) {
        attempts.add(trial);
        trial.check(code);
        return trial.getCheckResult();
    }

    private List<CheckPin> getLatestResult() {
        PinRow pinRow = attempts.get(attempts.size() - 1);
        pinRow.check(code);
        return pinRow.getCheckResult();
    }

    public boolean isSolved() {
        return code.getWidth() == getLatestResult().stream().filter(CheckPin.BLACK::equals).count();
    }

    public int getCodeWidth() {
        return code.getWidth();
    }

    public PinRow getCode() {
        return code;
    }


    public int getMaxAttempts() {
        return maxAttempts;
    }

    public boolean isAttemptLimitReached() {
        return attempts.size() < maxAttempts;
    }
}

ColorPin

package com.github.martinfrank.mastermind;

import java.util.*;

public enum ColorPin {

    RED, GREEN, BLUE, YELLOW, PURPLE, WHITE, ORANGE, GRAY, BLACK, CYAN;

    public static Set<ColorPin> randomSet(int amountColors) {
        List<ColorPin> all = Arrays.asList(ColorPin.values());
        Collections.shuffle(all);
        return new HashSet<>(all.subList(0, amountColors));
    }

    public static int maxWordLength(){
        return Arrays.stream(values()).mapToInt(c -> c.toString().length()).max().orElse(0);
    }
}

CheckPin

package com.github.martinfrank.mastermind;

public enum CheckPin {

    WHITE,  //proper color, wrong location
    BLACK; //proper color, proper location

}

PinRow

package com.github.martinfrank.mastermind;

import java.util.*;
import java.util.stream.IntStream;

public class PinRow {

    private final int width;
    private final ColorPin[] colors;
    private List<CheckPin> checkResult = new ArrayList<>();

    public PinRow(int[] indices, Set<ColorPin> givenColors) {
        List<ColorPin> sampleColors = new ArrayList<>(givenColors);
        width = indices.length;
        colors = new ColorPin[width];
        IntStream.range(0, width).forEach(c -> colors[c] = sampleColors.get(indices[c]));
    }

    public static PinRow randomPinRow(int codeWidth, Set<ColorPin> colors) {
        int[] indices = new int[codeWidth];
        int amountColors = colors.size();
        IntStream.range(0, codeWidth).forEach(c -> indices[c] = new Random().nextInt(amountColors));
        return new PinRow(indices, colors);
    }


    public List<CheckPin> check(PinRow code) {
        checkResult.clear();
        List<ColorPin> myRemaining = new ArrayList<>();
        List<ColorPin> codeRemaining = new ArrayList<>();
        for (int i = 0; i < width; i++) {
            ColorPin current = colors[i];
            ColorPin expected = code.colors[i];
            if (current == expected) {
                checkResult.add(CheckPin.BLACK);
            } else {
                myRemaining.add(current);
                codeRemaining.add(expected);
            }
        }

        for (ColorPin mine : myRemaining) {
            for (ColorPin candidate : codeRemaining) {
                if (mine.equals(candidate)) {
                    codeRemaining.remove(candidate);
                    checkResult.add(CheckPin.WHITE);
                    break;
                }
            }
        }
        return checkResult;
    }


    @Override
    public String toString(){
        return Arrays.toString(colors);
    }

    public List<CheckPin> getCheckResult() {
        return checkResult;
    }

    public List<ColorPin> getColorPins() {
        return Arrays.asList(colors);
    }

    @Override
    public boolean equals(Object o) {
        if (this == o) return true;
        if (o == null || getClass() != o.getClass()) return false;
        PinRow pinRow = (PinRow) o;
        return Arrays.equals(colors, pinRow.colors);
    }

    @Override
    public int hashCode() {
        return Arrays.hashCode(colors);
    }

    public int getWidth() {
        return width;
    }

}

PinChecker

package com.github.martinfrank.mastermind;

import java.util.List;

public class PinChecker {

    private final List<CheckPin> reference;

    public PinChecker(List<CheckPin> reference) {
        this.reference = reference;
    }

    public boolean isSuperior(List<CheckPin> candidate){
        if( candidate.size() < reference.size()){
            return false;
        }
        if( candidate.size() == reference.size()){
            long baseBlack = reference.stream().filter(cp -> cp==CheckPin.BLACK).count();
            long candidateBlack = candidate.stream().filter(cp -> cp==CheckPin.BLACK).count();
            return candidateBlack <= baseBlack;
        }
        return false;
    }

}

BoardSolver

package com.github.martinfrank.mastermind;

import java.util.*;
import java.util.stream.Collectors;

public class BoardSolver {

    private final Board board;
    private final Set<ColorPin> colors;
    private final Random random;

    public BoardSolver(Board board, Set<ColorPin> colors) {
        this.board = board;
        this.colors = colors;
        random = new Random();
    }

    public void solve() {
        int codeWidth = board.getCodeWidth();
        List<PinRow> combinations = PinRowCombinations.getAll(colors, codeWidth);
        do {
            PinRow attempt = removeRandom(combinations);
            PinChecker pinChecker = new PinChecker(board.addAttempt(attempt));
            combinations = combinations.stream().filter(pr -> pinChecker.isSuperior(pr.check(attempt))).collect(Collectors.toList());
        } while (!board.isSolved() && board.isAttemptLimitReached());

    }

    private PinRow removeRandom(List<PinRow> combinations) {
        return combinations.remove(random.nextInt(combinations.size()));
    }

}

PinRowCombinations

package com.github.martinfrank.mastermind;

import java.math.BigInteger;
import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.stream.IntStream;

public class PinRowCombinations {

    public static List<PinRow> getAll(Set<ColorPin> colors, int width) {
        int amountColors = colors.size();
        int size = (int)Math.pow( amountColors, width);

        List<PinRow> result = new ArrayList<>();
        for (int i = 0; i < size; i ++){
            BigInteger bi = BigInteger.valueOf(i);
            int[] indices = createIndice(leadingZeros(bi.toString(amountColors), width) );
            result.add(new PinRow(indices, colors));
        }
        return result;
    }

    private static String leadingZeros(String numberString, int size) {
        StringBuilder sb = new StringBuilder();
        IntStream.range(0,size-numberString.length()).forEach(i -> sb.append("0"));
        sb.append(numberString);
        return sb.toString();
    }

    private static int[] createIndice(String numberString) {
        int length = numberString.length();
        int[] indice = new int[length];
        IntStream.range(0,length).forEach(i -> indice[i] = createIndex(numberString.charAt(i)));
        return indice;
    }

    private static int createIndex(char charAt) {
        if (charAt >= '0' && charAt <= '9'){
            return charAt-'0';
        }else{
            return charAt-'a'+10;
        }
    }
}

BoardPrinter

package com.github.martinfrank.mastermind;

import java.util.Arrays;
import java.util.List;
import java.util.stream.IntStream;

public class BoardPrinter {

    public static void print(Board board){
        final int columnSize = ColorPin.maxWordLength();
        final int codeWidth = board.getCodeWidth();
        final PinRow code = board.getCode();

        String separator = createSeparator(columnSize+2, codeWidth);
        String bigSeparator = createSeparator(columnSize+2, codeWidth, '=');
        System.out.println((board.isSolved()?"":"not ")+"solved in "+board.getAttempts().size()+" turns...");
        System.out.println(separator);
        System.out.println(printRow(code, columnSize)+emptyPins(codeWidth, columnSize));
        System.out.println(bigSeparator);
        for(int i = board.getMaxAttempts()-1; i >= 0; i --){
            if(i < board.getAttempts().size() ){
                PinRow trial = board.getAttempts().get(i);
                System.out.println(printRow(trial, columnSize)+printCheck(trial.getCheckResult(), codeWidth, columnSize));
            }else{
                System.out.println(emptyPins(codeWidth, columnSize)+emptyPins(codeWidth, columnSize));
            }
            System.out.println(separator);
        }

    }

    private static String emptyPins(int codeWidth, int columnSize) {
        StringBuilder sb = new StringBuilder("|");
        IntStream.range(0,codeWidth).forEach(i -> append(sb, " ", centerString("", columnSize), " |"));
        return sb.toString();
    }

    private static String createSeparator(int columsSize, int codeWidth) {
        return createSeparator(columsSize, codeWidth, '-');
    }

    private static String createSeparator(int columsSize, int codeWidth, char c) {
        StringBuilder sb = new StringBuilder("+");
        String stroke = createStroke(columsSize, c);
        IntStream.range(0,codeWidth).forEach(i -> append(sb,stroke,"+"));
        sb.append("+");
        IntStream.range(0,codeWidth).forEach(i -> append(sb,stroke,"+"));
        return sb.toString();
    }

    private static String createStroke(int strokeLength, char c) {
        StringBuilder sb = new StringBuilder();
        IntStream.range(0,strokeLength).forEach(i -> sb.append(c));
        return sb.toString();
    }

    private static void append(StringBuilder sb, String... values){
        Arrays.stream(values).forEach(sb::append);
    }

    private static String printCheck(List<CheckPin> checkers, int codeWidth, int columnLength) {
        StringBuilder sb = new StringBuilder("|");
        for(int i = 0; i < codeWidth; i++){
            String value = i < checkers.size()?checkers.get(i).toString():" ";
            append(sb, " ",centerString(value, columnLength)," |");
        }
        return sb.toString();
    }

    private static String printRow(PinRow row, int columnLength) {
        StringBuilder sb = new StringBuilder("|");
        row.getColorPins().forEach(cp -> append(sb, " ",centerString(cp.toString(), columnLength)," |"));
        return sb.toString();
    }

    public static String centerString (String string, int width) {
        return String.format("%-" + width  + "s", String.format("%" + (string.length() + (width - string.length()) / 2) + "s", string));
    }
}

MasterMind

package com.github.martinfrank.mastermind;

import java.util.Set;

public class MasterMind {


    public static void main(String[] args){
        final int codeWidth = 4;
        final Set<ColorPin> colors = ColorPin.randomSet(6);
        final int maxAmountTrials = 20;
        final PinRow code = PinRow.randomPinRow(codeWidth, colors);

        Board board = new Board(code, maxAmountTrials);
        BoardSolver solver = new BoardSolver(board, colors);
        solver.solve();
        BoardPrinter.print(board);
    }

}

Accepted Answer

I've been sick for a few days so i was not able to react any sooner. It's hard for me to pick the right answer out of these here, they are all really helpful and every answer discovers more improvements on my code. So I can't pick one and accept it without disrespecting the answers of the others. That's why i don't accept one but all answer here!!!

Answer 1

Pretty good code in general. Well formatted and easy to read.

Board

I feel like the game constraints should exist in some common location. In this case the Board should know how many tries there are and how many pins each row may contain. Now it is possible to add different sized PinRows to a Board and no checking is performed. A trial shorter than the code results in an error.

In the addAttempt(...) method the responsibility of checking the code is placed on the incoming PinRow. This provides an opportunity for the caller to inject a subclass of PinRow with an overridden check(...) method that records the code and solve the problem in two tries via cheating. I would move the code checking into a separate class, as I consider the PinRow to be a "dumb" data container.

In general, I would make the representation of the board as simple as possible and place the game logic into a separate game engine.

ColorPin

Hard coded UI colours. Allowing unlimited pin row length would logically suggest that unlimited colors are also allowed. Better leave the colours for the UI and just use integers here.

CheckPin

This blends UI code again to the game logic. The enums should just be named "COLOR_IN_CORRECT_LOCATION" an "COLOR_IN_WRONG_LOCATION" instead of making the programmer remember what the colours actually mean. A command line UI probably wouldn't have colours anyway.

PinRow

The "colors" field should be named "pins", because it is a row of pins.

Replacing for-loops with IntStreams isn't my cup of tea. The code is not easily understandable at all. I would like to see JavaDocs for the constructor parameters as they are not very intuitive. To create a PinRow I would expect to be required to just provide a number of ColorPin references that matches the size of the row. I assume you chose this constructor to make it easier to create the combinations in the solver. If you're implementing a Master Mind game, it's API should reflect the use cases needed when playing and the solver should adapt to the API, not the other way around.

The randomPinRow() method hard codes the game logic into the data object. For example it is now not very easy to switch to game rules that allow empty pins. This could be extracted into a separate class that implements Supplier<PinRow>. A devious "code master" would learn the "code breaker's" habits and that could be implemented in the supplier.

Split the PinRow and CheckResult into separate classes to follow single responsibility principle and introduce a new PlayerGuess to connect them.

PinChecker

The PinChecker is essentially a Comparator<CheckResult>. You should use standard library tools when possible.

Answer 2

This is a very nice project, some suggestions for you:

Board

You have the following code:

public class Board {
    private final int maxAttempts;
    private final PinRow code;
    private final List<PinRow> attempts = new ArrayList<>();
    public Board(PinRow code, int maxAmountTrials) {
        this.code = code;
        this.maxAttempts = maxAmountTrials;
    }
}

You can put initialization of field attempts inside the constructor:

public class Board {

    private final int maxAttempts;
    private final PinRow code;
    private final List<PinRow> attempts;

    public Board(PinRow code, int maxAmountTrials) {
        this.code = code;
        this.maxAttempts = maxAmountTrials;
        this.attempts = new ArrayList<>();
    }
}

PinRow

You have the following method:

public static PinRow randomPinRow(int codeWidth, Set<ColorPin> colors) {
        int[] indices = new int[codeWidth];
        int amountColors = colors.size();
        IntStream.range(0, codeWidth).forEach(c -> indices[c] = new Random().nextInt(amountColors));
        return new PinRow(indices, colors);
}

You can reduce method using Random.ints:

 public static PinRow randomPinRow(int codeWidth, Set<ColorPin> colors) {
        int[] indices = new Random().ints(codeWidth, 0, colors.size()).toArray();
        return new PinRow(indices, colors);
 }

PinRowCombinations

You have the following methods:

private static String leadingZeros(String numberString, int size) {
        StringBuilder sb = new StringBuilder();
        IntStream.range(0,size-numberString.length()).forEach(i -> sb.append("0"));
        sb.append(numberString);
        return sb.toString();
}
private static int[] createIndice(String numberString) {
    int length = numberString.length();
    int[] indice = new int[length];
    IntStream.range(0,length).forEach(i -> indice[i] = createIndex(numberString.charAt(i)));
    return indice;
}
private static int createIndex(char charAt) {
    if (charAt >= '0' && charAt <= '9'){
          return charAt-'0';
    } else{
          return charAt-'a'+10;
    }
}

The methods can be rewritten using Collections and a chars stream:

private static String leadingZeros(String numberString, int size) {
    return String.join("", Collections.nCopies(size - numberString.length(), "0")) + numberString;
}

private static int[] createIndice(String numberString) {
    return numberString.chars().map(i -> createIndex((char) i)).toArray();
}

private static int createIndex(char charAt) {
    if (Character.isDigit(charAt)) { return charAt - '0';}
     return charAt - 'a'+ 10;    
}

BoardPrinter

Again the Collections.nCopies method so you can write:

private static String createStroke(int strokeLength, char c) {
    StringBuilder sb = new StringBuilder();
    IntStream.range(0,strokeLength).forEach(i -> sb.append(c));
    return sb.toString();
}

The new version of the method is:

private static String createStroke(int strokeLength, char c) {
    return String.join("", Collections.nCopies(strokeLength, Character.toString(c)));
}

Probably in the class BoardPrinter the method nCopies could be used to abbreviate some part of the code, and I suspect that in other classes some method about comparison could be deleted maybe implementing some compare interface, anyway the project is well structured and articulate.

Answer 3

The BoardSolver can be initialized with a different set of colors than the code, which would lead in an unsolvable solution which is not correct because the board is solvable:

final Set<ColorPin> colors = /* red, blue*/;
final Set<ColorPin> otherColors = /* yellow, green */;

final PinRow code = PinRow.randomPinRow(codeWidth, colors);
Board board = new Board(code, maxAmountTrials);
BoardSolver solver = new BoardSolver(board, otherColors);

For each Board a new BoardSolver is needed:

Board board = new Board(/* ... */);
Board otherBoard = new Board(/* ... */);

BoardSolver solver = new BoardSolver(board, ...);
BoardSolver solver = new BoardSolver(otherBoard, ...);

I think a BoardSolver does not need to depend on a specific Board by its construction. Instead we should pass the board as an argument to solve. Since the Board knows code which knows colors the BoardSolver does not need its own colors because it knows the colors through the Board:

public static void main(String[] args) {
  /* ... */
  BoardSolver solver = new BoardSolver();
  Board board = new Board(code, maxAmountTrials);

  solver.solve(board);
  BoardPrinter.print(board);
}

Personally I find it cleaner if solve would return a Board:

public static void main(String[] args) {
  /* ... */
  Board solved = solver.solve(board);
  BoardPrinter.print(solved);
}

---

The check method in PinRow returns a List that contains BLACK and WHITE pins:

public List<CheckPin> check(PinRow code) {

 /* add BLACK to checkreult */

 /* add WHITE to checkreult */>

 return checkResult;
}

To check if you solved the game a loop through the List is needed to check if an attempt is correct:

public boolean isSolved() {
   return code.getWidth() == getLatestResult().stream().filter(CheckPin.BLACK::equals).count();
}

When you check, you could store the number of BLACK and WHITE pins instead of instances of them in a List.

For instance when you write a shopping list you do not write it this way:

milk
milk
eggs
milk
eggs

Now you have to count (loop like in isSolved) through the list to find the amount of milk and eggs

We can save directly the amounts of BLACK and WHITE:

public Resultcheck(PinRow code) {
  int amountOfBlack;
  int amoutOfWhite;
  for (int i = 0; i < width; i++) {
     /* amountOfBlack++ */
     /* amoutOfWhite++ */
  }
  return new Result(amountOfBlack, amoutOfWhite);
}

public boolean isSolved() {
    return code.getWidth() == getLatestResult().amountOfBlack;
}