(显然)缺乏反馈。在尝试编写该程序时,我发现自己可以解决试错法和临时解决方案遇到的大多数问题,我担心这些问题可能会导致一些偶然的代码。一个很好的例子是几个变量的初始化:
bestline
类中的bestrow
和Testing
,以及bestMove
类中的Board
。代码也感觉很麻烦。如果您可以查看我的代码并评论我可以改进的地方,应该采用的最佳做法等,我将不胜感激。/>
import java.util.Scanner;
public class Testing
{
public static void main(String[] args) {
int turn = 0; // turn is either 0 or 1 , changed with Math.abs(turn-1).
Board board = new Board ();
board.initializeBoard(board);
Scanner myScanner = new Scanner(System.in);
for( ; board.scoreBoard(board) == 1000 ; ){
if (turn == 0){ //X's turn
int line = myScanner.nextInt(); // get a line
int row = myScanner.nextInt(); // get a row
for ( ; board.Square[line][row] != 0 ;line = myScanner.nextInt() , row = myScanner.nextInt() ){ // run while the square at [line][row] is taken,print the line and pick another.
System.out.println("Square Taken , please pick another.");
}
board.Square[line][row] = 1; //make the move (1 means X)
board.printBoard(board);
}
if (turn == 1){ //O's turn
int lastboard = 100; // O's worst case scenario so it has somthing to compare in the first round. // this will keep the best score that player can get
int bestline=0; // no meaning because I had to initialize
int bestrow =0; // no meaning because I had to initialize
for(int line = 0 ; line <3 ; line++){ //
for(int row = 0 ; row < 3 ; row ++){ //go over the entire board
if(board.Square[line][row]==0){ //if (Square is empty)
board.Square[line][row] = 2; // place 2 (2 means O) , this "tries" a move.
int tmpboard = board.scoreBestMove(board, Math.abs(turn-1)); //"scores" the move just made, keeps it in a temporary int.
if(tmpboard<=lastboard){ // if the move just made is better than previous best move.
lastboard = tmpboard; // keep the new score as the best score.
System.out.println(line+" "+row+" this is " +board.scoreBestMove(board, Math.abs(turn-1))); // printing how it scores every move for debugging. // ignore.
bestline = line; // keeps the best moves line.
bestrow = row; // keeps the best moves row.
}
board.Square[line][row] = 0; // resets the square.
}
}
}
board.Square[bestline][bestrow] = 2; // once the loop is over , do the best move you found.
board.printBoard(board); // print the board.
}
turn = Math.abs(turn-1); // change the turn.
}
System.out.println(board.scoreBoard(board));
}
}
#1 楼
欢迎使用CodeReview。您对代码质量的关注是有根据的,但不要让您失望:自己深入研究该书,编写自己的代码,甚至将其暴露给Internet的批评者,都是朝着正确方向迈出的令人印象深刻的第一步。您似乎对编写简洁的代码感兴趣。我可以热烈推荐《清洁代码:敏捷软件技巧手册》(Robert C. Martin)。对我而言,这是一本非常发人深省的书,它帮助我真正地了解了内部软件质量为何重要(以及如何实现)的原因。 。 (正如他指出的那样,您可能想重新讨论对象实例和
this
关键字的概念。)除了已经指出的要点之外,我还要解释一些真正影响代码质量的抽象主题。 理论背景
基本问题
程序员喜欢认为自己很聪明。我们倾向于解决一个问题,惊奇于我们的聪明,甚至在没有意识到的情况下就留下一团糟。只有在稍后我们重新访问代码时(通常是在尝试添加功能或修复错误的过程中),我们才意识到阅读和理解代码比编写代码要困难得多。经常有太多信息无法轻松地绕开我们的头脑。
然后我们写评论(在您的情况下,每行几乎是一条评论)来帮助您理解整个事情,但是这样做,我们会添加更多信息,这一次甚至无法保证可靠性或真实性:注释的生成速度比40°C的冰箱中遗留的牛奶快,从而使其中包含的“信息”已过时。
更糟的是,注释是通常是多余的。好像我们写了两次一样!一个好的程序员是一个懒惰的程序员,因为她从不重复自己。
(评论确实有其位置,例如在公共API的文档中,以及解释我们做出的特别奇怪的决定。)
解决方案
为了应对复杂性,我们需要将其分解并简化越远越好。换句话说:分而治之。如果我们的班级很小,我们很容易发现职责。如果我们的方法很小,我们可以很容易地辨别出控制流程。如果我们的行很短,我们就可以理解其中包含的语句。
简洁不是唯一重要的事情。选择专有名称甚至更重要。如果没有专有名称,我们将不断解码和重构仅通过阅读代码即可获得的明显信息。您的
main
方法。它跨越近50行,包含六个嵌套级别。我真的很努力,但是我的头几乎爆炸了,试图理解您在那所做的一切。如果该方法看起来像这样呢?
private static final Scanner in = new Scanner(System.in);
private static final PrintStream out = System.out;
private static final Game game = new Game();
public static void main(String args[]) {
out.println("Welcome to TicTacToe.");
out.println(game);
while (game.isRunning()) {
informPlayersOfNextTurn();
makeNextMove();
out.println(game);
}
out.println(game.getResult());
}
投入了大量的精力进行了转换将您的原始代码转换成这种易于理解的表达形式。但是,您问,所有代码都去了哪里?让我们一步一步来。
因为我已将所有内容拆分为单独的方法,每个方法仅执行一项操作,所以我将局部变量提升为
private static final
字段。有您熟悉的扫描仪,以及对System.out
的引用,因此我可以使用缩写形式out.println(...)
并放下System
(我觉得它更方便,但请按照您最喜欢的方式进行。) main
看起来都非常符合您的期望:private static void informPlayersOfNextTurn() {
String message = "Player %s, it's your turn. Make your move!\n";
out.printf(message, game.getCurrentPlayer());
}
我们只是向游戏询问当前玩家,并提示他们采取行动。
注意这些方法调用的高层次...这里没有太多的实现可见,只是对程序运行方式的高层次视图。在用户输入有效的密码之前,我们会一直要求移动:
private static void makeNextMove() {
Move move = askForMove();
while (!game.isMoveAllowed(move)) {
out.println("Illegal move, try again.");
move = askForMove();
}
game.makeMove(move);
}
现在,我们进行更深入的研究。用户通常不习惯从零开始计数的编程约定,因此我们将允许他们以熟悉的方式输入其移动并减去
1
以将其转换为适合Java的格式。private static Move askForMove() {
int x = askForCoordinate("horizontal");
int y = askForCoordinate("vertical");
return new Move(x - 1, y - 1);
}
最后,我们到达了实现级别:我们正在使用扫描仪来收集一些有效的输入。
private static int askForCoordinate(String coordinate) {
out.printf("Enter the %s coordinate of the cell [1-3]: ", coordinate);
while (!in.hasNextInt()) {
out.print("Invalid number, re-enter: ");
in.next();
}
return in.nextInt();
}
就这样!这就是
Testing
类中的全部内容。 (我将其称为Main
是因为这是我的个人习惯,但是Testing
也可以)。其余的代码又发生了什么?在
Main
中,我们实际上仅对逻辑的粗糙流程和处理输入感兴趣,因此其他职责已委托给Game
。原始代码的问题之一是它只有两个类:Testing
和Board
。但是,这些责任捆绑在一起太多了。 br /> public class Game {
private final Board board = new Board();
private Player currentPlayer = Player.X;
向上滚动并查看
Board
中的while循环。因此,现在很清楚:只有在没有赢家并且棋盘尚未满员的情况下,游戏才能运行。否则,它就结束了。 public boolean isRunning() {
return !currentPlayer.isWinner && !board.isFull();
}
如果游戏正在运行并且我们要放入X或O的单元格为空,则允许移动。游戏仍在运行。如果我们在移动后没有获胜者,则开始下一个玩家的回合:并将
Player
打印到控制台上,将其覆盖(因此Player
将在当前状态下打印游戏板): public Player getCurrentPlayer() {
return currentPlayer;
}
public boolean isMoveAllowed(Move move) {
return isRunning() && board.isCellEmpty(move.X, move.Y);
}
现在要看的唯一主要的东西是
main
。 />井字游戏板 public void makeMove(Move move) {
if (!isRunning()) {
throw new IllegalStateException("Game has ended!");
}
board.setCell(move.X, move.Y, currentPlayer);
if (!currentPlayer.isWinner) {
currentPlayer = currentPlayer.getNextPlayer();
}
}
我不是使用
toString
的“二维”数组,而是使用out.println(game);
枚举来使代码更具表现力(并摆脱了所有魔术。原始代码中普遍使用的数字,例如500、1000和50)。我们将到目前为止的移动次数进行计数,以方便地判断板是否已满,而无需稍后计算完整的单元格。在构造函数中,我们用单元格填充所有行:
public GameResult getResult() {
if (isRunning()) {
throw new IllegalStateException("Game is still running!");
}
return new GameResult(currentPlayer);
}
以下玩家移动时调用方法。它取代了条件跨越多行的三个巨大的if语句。即使在此重构版本中,仍然存在一些复杂性。基本上,方法是:
我们可以基于当前移动进行计算,
所以我们只需要检查当前列,行,对角线和反对角线。
如果其中任何一个足够长(
Board
),我们就有赢家。如果这没有道理,请在一块纸上画板,然后用铅笔穿过它。
@Override
public String toString() {
return board.toString();
}
}
我发现最简单的方法是查找二进制值,该值是否等于
int
是二进制搜索,仅在对长度进行排序时有效。您也可以使用循环。在游戏中,有时我们必须检查一个单元是否为空,因此我们要检查它是否在棋盘上,如果是,则是否也为空白:
public class Board {
private final int LENGTH = 3;
private final Player[][] cells = new Player[LENGTH][LENGTH];
private int numberOfMoves = 0;
九步移动后,棋盘总是注满:
public Board() {
for (Player[] row : cells)
Arrays.fill(row, Player.Blank);
}
最后,我们再次重写
Player
,从而使我们能够输出游戏棋盘。这仍然比我想要的复杂得多。理想情况下,可以将其委托给单独的3
类。 public void setCell(int x, int y, Player player) {
cells[x][y] = player;
numberOfMoves++;
int row = 0, column = 0, diagonal = 0, antiDiagonal = 0;
for (int i = 0; i < LENGTH; i++) {
if (cells[x][i] == player) column++;
if (cells[i][y] == player) row++;
if (cells[i][i] == player) diagonal++;
if (cells[i][LENGTH - i - 1] == player) antiDiagonal++;
}
player.isWinner = isAnyLongEnough(row, column, diagonal, antiDiagonal);
}
private boolean isAnyLongEnough(int... combinationLengths) {
Arrays.sort(combinationLengths);
return Arrays.binarySearch(combinationLengths, LENGTH) >= 0;
}
就是这样!好吧,差不多。您可能已经很好地了解了枚举
LENGTH
和帮助程序类toString
的外观,但是为了完整起见,我将向他们展示。帮助程序数据结构
一些Java专家和专业人士会告诉您,每个领域都应受到getter和setter的保护。我不同意(罗伯特·C·马丁也持相同观点):有些类确实没有重要的保护状态,因此应将其分类为没有行为的数据结构,并且可能具有公共领域。其他人会不同意,并且使用getter-setter方法也很好。给我们一个空白。另外,还有一种方便的方法可以给我们下一个玩家。
格式化游戏结果
public boolean isCellEmpty(int x, int y) {
boolean isInsideBoard = x < LENGTH && y < LENGTH && x >= 0 && y >= 0;
return isInsideBoard && cells[x][y] == Player.Blank;
}
玩家移动的数据结构
public boolean isFull() {
return numberOfMoves == LENGTH * LENGTH;
}
我希望您觉得这篇评论有用。我试图根据我的观点清楚地标记零件。花了几个小时的时间,我很高兴以投票和评论的形式提供一些反馈。 。我会尝试回来并在一段时间内添加一些链接。
评论
\ $ \ begingroup \ $
谢谢您的回答!我真的很感谢您的时间和精力,这确实使它更上一层楼。先生,您应该获得一枚奖牌。再次,这是一个比我以前想像的更好,更连贯和建设性的评论。谢谢!
\ $ \ endgroup \ $
–汤姆
2012年9月26日9:17
\ $ \ begingroup \ $
@谢谢汤姆。我在回答中忽略的一件事是实施中的人工智能。很高兴您发现该评论仍然很有用。
\ $ \ endgroup \ $
–亚当
2012年9月27日在20:40
#2 楼
这是您应该对Board
类的开头进行的一些简单修复。我让别人来解决其他问题。public class Board {
int Square[][] = new int [3][3];
Board(){
int Square[][] = new int [3][3];
}
// constructor for a new board , makes an int array size 3/3.
public void initializeBoard (Board board){
for(int i=0 ; i<3 ; i++)
for(int j=0 ; j<3 ; j++)
Square[i][j] = 0;
}
在上面的代码中,请执行以下操作:变量名称。
消除不需要的构造函数中的数组构造。由于您在此处声明它,因此它超出了构造函数结尾的范围。它会暂时隐藏您的同名实例变量,但不会被引用,也没有必要。
不要将板子传递到
initializeBoard()
,因为您没有使用它并且容易混淆。考虑将
initializeBoard
的代码移到Board()
中。问自己:是否有时间我想要一个未初始化的板?如果答案是否定的,则不需要调用方对其进行初始化。考虑消除
initializeBoard
代码。如您所见,Java中的基本类型具有默认值。对于int,该值为0。因此,将方阵的每个元素设置为0都不会执行任何操作。结果代码如下所示:
由于我将变量的名称从“ Square”更改为“ square”,因此您需要在其余代码中修复引用。
也请考虑引入
enum
s在代码中输入魔术常数,然后改用它们。当然,我们必须重新访问上面的Board
代码。而是使用当前实例的面板。我也将其更改为scoreBoard
,以更好地反映其目的。public class Board {
int square[][] = new int[3][3];
Board() {}
}
还将一些代码重构为较小的部分。例如,在
getScore()
类上创建以下函数。public enum SquareValue {
Blank,
X,
O
}
public enum BoardValue {
X_Wins(100),
O_Wins(0),
Draw(50),
GameNotOver(1000);
public int value;
BoardValue(int value) {
this.value = value;
}
}
您也可以使用其他功能来分解
Board
。如果您不希望从此类外部调用这些函数,请将它们设置为私有或受保护。看起来像这样:public class Board {
SquareValue[][] square = new SquareValue[3][3];
Board() {
for (int i=0; i<3; i++) {
for (int j=0; j<3; j++) {
square[i][j] = SquareValue.Blank;
}
}
}
}
评论
\ $ \ begingroup \ $
您还需要isWinningDiagonal()
\ $ \ endgroup \ $
– Jared Paolin
2012-09-25 18:34
\ $ \ begingroup \ $
当然。这并不意味着是完整的。我可能将其命名为hasWinningDiagonal(SquareValue value),因为要检查2条对角线。有很多方法可以将大scoreBoard方法切片和切块。
\ $ \ endgroup \ $
– andykellr
2012-09-25 18:49
\ $ \ begingroup \ $
很高兴您发现它很有帮助。玩得开心!
\ $ \ endgroup \ $
– andykellr
2012年9月25日19:00
\ $ \ begingroup \ $
哇!谢谢你的回答!这使代码更具可读性,并且实际上还帮助我理解了我没有完全掌握的一些概念。我没想到会有这么详细的答案=)。
\ $ \ endgroup \ $
–汤姆
2012-09-25 19:05
#3 楼
关于Board.scoreBoard(...)方法的快速提示:由于所有的行,很难阅读。
有一堆重复的代码。
您好像在使用&而不是&&-&是位移位运算符。
希望我没有误解该方法中的代码,但是这是另一种书写方式(IMHO :)更清晰。此处的代码比您的代码慢,但是在Java中,除非您正在编写实时代码(即机器人技术),否则清除总是比速度重要,实际上,即使如此,它仍然更重要。
public int scoreBoard(Board board) {
if( checkLines(board, 1) ) {
return 100;
}
else if( checkLines(board, 2) ) {
return 0;
}
for( int a = 0; a < 3; ++a ) {
for( int b = 0; b < 3; ++b ) {
if( board.Square[a][b] == 0 ) {
return 1000;
}
}
}
return 50;
}
private boolean checkLines(Board board, int player ) {
for( int x = 0, r = 0; r < 3; ++r, x = 0) {
if( board.Square[r][x] == player && board.Square[r][++x] == player && board.Square[r][++x] == player ) {
return true;
}
x = 0;
if( board.Square[x][r] == player && board.Square[++x][r] == player && board.Square[++x][r] == player ) {
return true;
}
}
if( board.Square[0][0] == player && board.Square[1][1] == player && board.Square[2][2] == player ) {
return true;
}
if( board.Square[0][2] == player && board.Square[1][1] == player && board.Square[2][0] == player ) {
return true;
}
return false;
}
评论
\ $ \ begingroup \ $
谢谢您的回答,似乎我没有注意到&-&&问题,现在我困惑为什么我的程序即使使用“&”而不是“ &&”仍能正常工作。
\ $ \ endgroup \ $
–汤姆
2012年9月26日上午9:24
\ $ \ begingroup \ $
哇,每天都有新变化:这证实了&是位移位运算符。但是,这个stackoverflow答案阐明了正如JLS的本节中所指定的,移位操作符确实可以使用布尔值!
\ $ \ endgroup \ $
–马可
2012-09-26 11:53
\ $ \ begingroup \ $
顺便说一句,使用&的效果是对整个行进行了评估,而不是在确定第一个(布尔)表达式为假时表示“ short-cutting”。
\ $ \ endgroup \ $
–马可
2012年9月26日上午11:59
评论
嗨,尽管递归scoreBestMove的一部分尚未得到审查,但仍获得了一些不错的答案,我知道乞g不能成为选择者,但我真的很希望对该函数进行审查。