应用程序

Question

我最近完成了一个Java应用程序，我想知道您是否可以对它进行代码检查，仅仅是为了了解我可以做得更好的事情:)

由于代码很长，我认为最好发布一个链接到我的github回购。

结构

码

作为一个提示，我取出了所有类的导入，除了控制台类，这些类使用第三方库只是为了缩短代码片段。

TicTacToeException

public class TicTacToeException extends Exception {
    public TicTacToeException(String message){
        super(message);
    }
}

播放器

public abstract class Player {
    private String symbol;

    public Player(String symbol){
        this.symbol = symbol;
    }

    public String getSymbol(){
        return symbol;
    }

    public abstract void makeMove(Board board) throws TicTacToeException;

    @Override
    public boolean equals(Object player){
        return this.symbol == ((Player) player).getSymbol();
    }
}

人类

public class Human extends Player {
    private int inputRow;
    private int inputCol;

    public Human(String symbol){
        super(symbol);
    }

    @Override
    public void makeMove(Board board) throws TicTacToeException {
        board.checkTile(inputRow, inputCol, this);
    }

    public void setMoveInput(int row, int column){
        inputRow = row;
        inputCol = column;
    }
}

电脑

public class Computer extends Player {
    private ComputerAI internalAI;

    public Computer(String symbol, ComputerAI ai){
        super(symbol);
        internalAI = ai;
    }

    @Override
    public void makeMove(Board board){
        internalAI.makeBestMove(board, this);
    }
}

瓷砖

/**
 * Represents a tile of the tic-tac-toe board.
 */
public class Tile {
    /**
     * The player who has checked the tile.
     */
    private Player player;

    public void check(Player player){
        this.player = player;
    }

    public void uncheck(){
        player = null;
    }

    public Boolean isChecked(){
        return player != null;
    }

    public String getCheck(){
        return isChecked() ? player.getSymbol() :  " ";
    }

    public Player getCheckingPlayer() {
        return player;
    }
}

板

public class Board {
    private static final int ROW_COUNT = 3;
    private static final int COLUMN_COUNT = 3;

    private Tile[][] board;

    public Board(){
        board = new Tile[ROW_COUNT][COLUMN_COUNT];

        for(int i = 0; i < ROW_COUNT; i++){
            for(int j = 0; j < COLUMN_COUNT; j++) {
                board[i][j] = new Tile();
            }
        }
    }

    public Tile getTile(int row, int column){
        return board[row][column];
    }

    public void checkTile(int row, int column, Player player) throws TicTacToeException {
        Tile tileToCheck = board[row][column];

        if(tileToCheck.isChecked()){
            throw new TicTacToeException("The chosen tile [" + (row+1) + ", " + (column+1) + "] is already checked.");
        }

        tileToCheck.check(player);
    }

    public Tile[] getAvailableTiles(){
        List<Tile> availableTiles = new ArrayList<Tile>();
        for(int i = 0; i < ROW_COUNT; i++){
            for(int j = 0; j < ROW_COUNT; j++){
                if(!board[i][j].isChecked()){
                    availableTiles.add(board[i][j]);
                }
            }
        }

        return availableTiles.toArray(new Tile[0]); // Just for the sake of returning always arrays.
    }

    public Tile[] getRow(int row){
        return board[row];
    }

    public Tile[] getColumn(int column){
        Tile[] tiles = new Tile[ROW_COUNT];

        for(int row = 0; row < ROW_COUNT; row++){
            tiles[row] = board[row][column];
        }

        return tiles;
    }

    public Tile[] getMainDiagonal(){
        return new Tile[]{ board[0][0], board[1][1], board[2][2] };
    }

    public Tile[] getAntiDiagonal(){
        return new Tile[]{ board[0][2], board[1][1], board[2][0] };
    }

    public int getRowCount(){
        return ROW_COUNT;
    }

    public int getColumnCount(){
        return COLUMN_COUNT;
    }

    public List<Player> getPlayers(){
        List<Player> players = new ArrayList<Player>();
        for(int i = 0; i < ROW_COUNT; i++){
            for(int j = 0; j < COLUMN_COUNT; j++){
                Player player = board[i][j].getCheckingPlayer();
                if(player != null && !players.contains(player)){
                    players.add(player);
                }
            }
        }

        return players;
    }

    public Boolean isBoardComplete(){
        for(int i = 0; i < ROW_COUNT; i++){
            for(int j = 0; j < COLUMN_COUNT; j++){
                Tile tile = board[i][j];
                if(!tile.isChecked()){
                    return false;
                }
            }
        }
        return true;
    }

    public Boolean isEmpty(){
        return getAvailableTiles().length == ROW_COUNT*COLUMN_COUNT;
    }

    public Boolean hasWon(Player player){
        Tile[] diagonal = getMainDiagonal();
        Tile[] antiDiagonal = getAntiDiagonal();
        if(areCheckedBySamePlayer(diagonal,player) || areCheckedBySamePlayer(antiDiagonal, player)){
            return true;
        }

        for(int i = 0; i <= 2; i++){
            Tile[] row = getRow(i);
            Tile[] column = getColumn(i);
            if(areCheckedBySamePlayer(row, player) || areCheckedBySamePlayer(column, player)){
                return true;
            }
        }

        return false;
    }

    /**
     * Util method to re-use the logic. //TODO: does it really belong here?
     * @param tiles tiles to check
     * @return Boolean
     */
    private static Boolean areCheckedBySamePlayer(Tile[] tiles, Player player){
        String marker = player.getSymbol();
        return marker == tiles[0].getCheck() && tiles[0].getCheck() == tiles[1].getCheck() && tiles[0].getCheck() == tiles[2].getCheck();
    }
}

游戏

public class Game {
    private Board board;
    private Player player1;
    private Player player2;
    private Player currentPlayer;

    public Game(Player player1, Player player2){
        this.player1 = player1;
        this.player2 = player2;

        // Default - player 1 goes first unless changed later.
        chooseInitialPlayer(player1);

        board = new Board();
    }

    public void chooseInitialPlayer(Player player){
        currentPlayer = player;
    }

    public void chooseInitialPlayer(String symbol) throws TicTacToeException {
        if(player1.getSymbol().equalsIgnoreCase(symbol)){
            chooseInitialPlayer(player1);
        }
        else if(player2.getSymbol().equalsIgnoreCase(symbol)){
            chooseInitialPlayer(player2);
        }
        else{
            throw new TicTacToeException("The input symbol doesn't correspond to any current player."
                    + "\n Player 1 is set as the default starting player.");
        }
    }

    public void changeTurn(){
        if(currentPlayer == player1){
            currentPlayer = player2;
        }
        else{
            currentPlayer = player1;
        }
    }

    public void makeMove(int row, int column) throws TicTacToeException {
        if(row > 3 || row < 1 || column > 3 || column < 1){
            throw new TicTacToeException("The row and column don't correspond to any coordinates in the board.");
        }

        ((Human) currentPlayer).setMoveInput(row-1, column-1);
        currentPlayer.makeMove(board);
    }

    public void makeMove() throws TicTacToeException {
        currentPlayer.makeMove(board);
    }

    public Boolean isBoardComplete(){
        return board.isBoardComplete();
    }

    public Boolean hasCurrentPlayerWon(){
        return board.hasWon(currentPlayer);
    }

    public Boolean isTie(){
        return isBoardComplete() && !hasCurrentPlayerWon();
    }

    public Player getCurrentPlayer() {
        return currentPlayer;
    }

    public Player[] getPlayers(){
        return new Player[]{ player1, player2 };
    }

    public Board getBoard() {
        return board;
    }
}

public interface ComputerAI {
    public void makeBestMove(Board board, Player player);
}

RulesAI

/**
 * Implements a rule-based algorithm to make the best possible move in a Tic-Tac-Toe board.
 * The approach taken is an implementation of the Chain-of-responsibility pattern.
 */
public class RulesAI implements ComputerAI {
    /**
     * Set of rules/strategies to decide the next move in priority order.
     */
    protected static final MoveStrategy[] STRATEGIES = {
            new WinStrategy(),
            new BlockWinStrategy(),
            new ForkStrategy(),
            new BlockForkStrategy(),
            new RandomMoveStrategy()
    };

    /**
     * Apply the different tile-checking strategies available in priority order in order to make the first
     * available move.
     * @param board the board in its current status.
     * @param currentPlayer the player to move next.
     */
    public void makeBestMove(Board board, Player currentPlayer){
        if(board.isEmpty()){
            // This would only be "the best move" in a 3x3 board.
            board.getTile(1,1).check(currentPlayer);
            return;
        }

        for(MoveStrategy strategy : STRATEGIES) {
            if (strategy.checkTile(board, currentPlayer)) {
                break;
            }
        }
    }
}

MoveStrategy

public abstract class MoveStrategy {
    public abstract boolean checkTile(Board board, Player player);

    protected boolean isWinningMove(Board board, Tile tileToCheck){
        Player currentPlayer = tileToCheck.getCheckingPlayer();

        if (board.hasWon(currentPlayer) || board.isBoardComplete()) {
            return true;
        }

        return false;
    }

    protected Player getOpponent(Board board, Player player){
        for(Player iteratedPlayer : board.getPlayers()){
            if(!iteratedPlayer.equals(player)){
                return iteratedPlayer;
            }
        }

        return null; //TODO - NotAPlayer class? throw exception?
    }
}

WinStrategy

/**
 * Strategy to mark an available tile only if it secures a win.
 */
public class WinStrategy extends MoveStrategy {
    @Override
    public boolean checkTile(Board board, Player player){
        Tile[] availableTiles = board.getAvailableTiles();

        for (Tile availableTile : availableTiles){
            availableTile.check(player);
            if(isWinningMove(board, availableTile)){
                return true;
            }

            availableTile.uncheck();
        }

        return false;
    }
}

RandomMoveStrategy

/**
 * Strategy to mark the first available tile in the board.
 * //TODO - It serves its purpose, but it's not really random.
 */
public class RandomMoveStrategy extends MoveStrategy{
    @Override
    public boolean checkTile(Board board, Player player) {
        Tile[] availableTiles = board.getAvailableTiles();

        for (Tile availableTile : availableTiles){
            availableTile.check(player);
            return true;
        }

        return false;
    }
}

ForkStrategy

/**
 * Strategy to mark an available tile if it opens a fork win condition.
 * def. Fork: opens two different win paths.
 */
public class ForkStrategy extends MoveStrategy {
    @Override
    public boolean checkTile(Board board, Player player) {
        Tile[] availableTiles = board.getAvailableTiles();
        int winConditions = 0;

        for (Tile availableTile : availableTiles){
            availableTile.check(player);
            Tile[] futureAvailableTiles = board.getAvailableTiles();

            for(Tile futureAvailableTile : futureAvailableTiles){
                futureAvailableTile.check(player);

                if(isWinningMove(board, availableTile)){
                    winConditions++;
                    if(winConditions > 1){
                        futureAvailableTile.uncheck();
                        return true;
                    }
                }

                futureAvailableTile.uncheck();
            }

            availableTile.uncheck();
            winConditions = 0;
        }

        return false;
    }
}

BlockWinStrategy

/**
 * Strategy to mark an available tile only if it blocks an opponent win.
 */
public class BlockWinStrategy extends MoveStrategy {
   @Override
    public boolean checkTile(Board board, Player player){
        Player opponent = getOpponent(board, player);
        if(opponent == null) return false;

        Tile[] availableTiles = board.getAvailableTiles();
        for (Tile availableTile : availableTiles){
            availableTile.check(opponent);
            if(isWinningMove(board, availableTile)){
                availableTile.check(player);
                return true;
            }
            availableTile.uncheck();
        }

        return false;
    }
}

BlockForkStrategy

/**
 * Strategy to mark an available tile only if it blocks an opponent fork option.
 * def. Fork: opens two different win paths.
 */
public class BlockForkStrategy extends MoveStrategy {
    @Override
    public boolean checkTile(Board board, Player player) {
        Player opponent = getOpponent(board, player);
        if(opponent == null) return false;

        Tile[] availableTiles = board.getAvailableTiles();
        int winConditions = 0;

        for (Tile availableTile : availableTiles){
            availableTile.check(opponent);
            Tile[] futureAvailableTiles = board.getAvailableTiles();

            for(Tile futureAvailableTile : futureAvailableTiles){
                futureAvailableTile.check(opponent);

                if(isWinningMove(board, availableTile)){
                    winConditions++;
                    if(winConditions > 1){
                        futureAvailableTile.uncheck();
                        availableTile.check(player);
                        return true;
                    }
                }

                futureAvailableTile.uncheck();
            }

            availableTile.uncheck();
            winConditions = 0;
        }

        return false;
    }
}

CliOptionsParser

import core.Game;
import core.ai.RulesAI;
import core.players.Computer;
import core.players.Human;
import core.players.Player;
import org.apache.commons.cli.*;

public class CliOptionsParser {
    public static Game parseOptions(String[] args) throws ParseException {
        CommandLineParser parser = new DefaultParser();
        Options cmdOptions = getCmdOptions();
        CommandLine commandLine = parser.parse(cmdOptions, args);

        String p1Marker = commandLine.getOptionValue("p1");
        Player player1 = commandLine.hasOption("p1IsComputer") ?
                new Computer(p1Marker, new RulesAI()) :
                new Human(p1Marker);

        String p2Marker = commandLine.getOptionValue("p2");
        Player player2 = commandLine.hasOption("p2IsComputer") ?
                new Computer(p2Marker, new RulesAI()) :
                new Human(p2Marker);

        return new Game(player1, player2);
    }

    private static Options getCmdOptions(){
        Options options = new Options();

        //CLI options.
        options.addRequiredOption("p1", "player1Token", true, "Player1's token/marker.");
        options.addRequiredOption("p2", "player2Token", true, "Player2's token/marker.");
        options.addOption("p1IsComputer", false, "Sets player1 as controlled by the AI.");
        options.addOption("p2IsComputer", false, "Sets player2 as controlled by the AI.");

        return options;
    }
}

应用程序

import core.Game;
import core.ai.RulesAI;
import core.board.Board;
import core.exceptions.TicTacToeException;
import core.players.Computer;
import core.players.Human;
import core.players.Player;
import org.apache.commons.cli.*;

import java.util.Scanner;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

public class Application {
    /**
     * Color constants to display the player symbols with different colours.
     */
    public static final String ANSI_RED = "\u001B[31m";
    public static final String ANSI_GREEN = "\u001B[32m";
    public static final String ANSI_RESET = "\u001B[0m";

    private static Scanner input = new Scanner(System.in);

    public static void main(String[] args) {
        Game game;

        // Initialize the game with its players and symbols from the CLI.
        try{
            game = CliOptionsParser.parseOptions(args);
        }
        catch (ParseException ex){
            System.out.println("An error has been found with the arguments: " + ex.getMessage());
            return;
        }

        chooseInitialPlayer(game);

        printGameBoard(game.getBoard());

        do {
            try {
                makeNextMove(game);
            }
            catch (Exception ex) {
                System.out.println(ex.getMessage());
                continue;
            }

            clearConsole();
            printGameBoard(game.getBoard());

            // Print end-game results.
            if(isGameEnded(game)) break;

            game.changeTurn();
        } while (!game.isBoardComplete());

        input.close();
    }

    private static void chooseInitialPlayer(Game game){
        System.out.print("Choose starting player: ");
        String symbol = input.nextLine();

        try {
            game.chooseInitialPlayer(symbol);
        }
        catch (TicTacToeException ex){
            System.out.println(ex.getMessage());
        }
    }

    private static int[] getHumanInput(){
        Scanner scanner = new Scanner(System.in);

        Matcher matcher;

        do{
            String input = scanner.next();

            String patternString = "^([0-9]),([0-9])$"; // NOTE: The number range (1-3) is being validated in the backend.
            Pattern pattern = Pattern.compile(patternString);
            matcher = pattern.matcher(input);

            if(!matcher.matches()){
                System.out.println("Invalid input. Please give the coordinates of the tile you want to check with the format \"row,column\"");
            }

        }while (!matcher.matches());

        return new int[]{ Integer.parseInt(matcher.group(1)), Integer.parseInt(matcher.group(2)) };
    }

    private static void makeNextMove(Game game) throws Exception{
        if (game.getCurrentPlayer() instanceof Human) {
            System.out.println("Enter the row and column of the tile you want to check:\n");
            int[] move = getHumanInput();
            game.makeMove(move[0], move[1]);

        } else {
            System.out.println("Please wait - the computer is thinking its next move.");
            game.makeMove();
            Thread.sleep(2000); // UX :-)
        }
    }

    private static boolean isGameEnded(Game game){
        if(game.hasCurrentPlayerWon()){
            System.out.println("Player " + game.getCurrentPlayer().getSymbol() + " has won!");
            return true;
        }
        if(game.isTie()){
            System.out.println("The game is tied!");
            return true;
        }

        return false;
    }

    private static void printGameBoard(Board board){
        String firstPlayer = board.getTile(0,0).getCheck();

        System.out.println(ANSI_RESET + "-------------");
        for(int i = 0; i < board.getRowCount(); i++){
            for(int j = 0; j < board.getColumnCount(); j++){
                System.out.print(ANSI_RESET + "| ");
                if(board.getTile(i, j).getCheck() == firstPlayer){
                    System.out.print(ANSI_GREEN + board.getTile(i, j).getCheck());
                }
                else{
                    System.out.print(ANSI_RED + board.getTile(i, j).getCheck());
                }
                System.out.print(" ");
            }
            System.out.print(ANSI_RESET + "|\n");
            System.out.println("-------------");
        }
    }

    private static void clearConsole() {
        System.out.print("\033[H\033[2J");
        System.out.flush();
    }
}

Answer 1

谢谢你分享你的代码。

公共游戏( Player player1，player player2){ this.player1 = player1；this.player2 = player2；// Default -player 1，除非稍后更改。chooseInitialPlayer(player1)；board =新板()；} chooseInitialPlayer( player ){ currentPlayer = player；}

为什么chooseInitialPlayer是公开的？

从非private或至少不是final的构造函数调用方法是危险的。它可以被子类覆盖，在此重写的子类方法中，您可以访问尚未初始化的子类的成员。这导致很难理解在子类构造函数中初始化的NPEs字段上的final。

另一方面，我怀疑这个方法是否需要从任何其他类中调用，而不是游戏本身。(我会为下一轮比赛创造一个新的游戏对象.)

公开无效changeTurn(){ if(currentPlayer == player1){ currentPlayer = player2；}player1{ currentPlayer = player1；}

这是一个直截了当的解决问题的方法，只有两个想法：

• 命名时，该方法不会改变一个转弯(不管这可能是什么.)。它改变了当前的播放器。因此，我把它命名为changeCurrentPlayer()。
• 替代实现如果你把你的播放器放在一个List<Player>里怎么办？在这种情况下，交换播放机应该是这样的: public void changeTurn(){ currentPlayer = players.remove(0)；// get first player从列表中；players.add(currentPlayer)；//将当前播放器作为最后一个}的好处是：
  • 更短
  • 不再需要变量player1和player2
  • 适用于任意数量的玩家。(好吧，这个游戏没有杀手.)

最后，我不鼓励使用数组，而是建议使用Collection类型。但也许那只是我..。

Answer 2

我看了一下console.Application类，看到了以下内容：

    Game game;

    // Initialize the game with its players and symbols from the CLI.
    try{
        game = CliOptionsParser.parseOptions(args);
    }
    catch (ParseException ex){
        System.out.println("An error has been found with the arguments: " + ex.getMessage());
        return;
    }

我认为在这里您应该避免将core.Game类的知识放在控制台包中。

我更喜欢看到这样的代码：

class Game {
    public Game(GameOptions options) { .... }
}

class GameOptions {
    public static GameOptions from(Options options) {
        GameOptions opts = new GameOptions();

        // validate options...

        return opts;
    }
}

class Application {
    public static void main(String args) {
        CliOptionsParser optionParser = new CliOptionsParser(args);

        Game game = new Game(GameOptions.from(optionParser));
        ...
    }
}

因此，您将所有初始化代码放在主类中，并将控制台与核心包分离。

关键是CliOptionsParse应该只关心解析命令行选项，而不是更多。

您应该把代码放在那里来初始化对象。

引入像GameOptions这样的特定类的原因是一个可选的，它允许将核心包与控制台包分离。

如果您决定使用此适配器类，请考虑创建一个适当的接口，以便将实现分隔开来。

GameOptions应该包含连接到游戏的选项验证代码。

例如，如果您只传递了一个格式正确的播放器，应该是GameOptions来抱怨缺少的播放器，而不是CliOptionsParser，这应该只关心命令行选项是否采用了正确的格式。

这有助于将业务逻辑放在适当的位置。