Simple Java Tic-Tac-Toe game

Question

About 2 weeks ago I made a Java game of Tic-Tac-Toe. It turned out great and it worked but it needed lots of help before it became a decent program. Since then, I have worked out lots of the kinks and errors. I would love to know what you think of my improved program.

public class ADitionalPractice {
public static boolean detection(boolean iswinning, char[][] board) {
 if (("" + board[0][0] + board[1][0] + board[2][0]).equals("xxx")) {
 iswinning = true;
 } else if (("" + board[1][0] + board[1][1] + board[2][1]).equals("xxx")) {
 iswinning = true;
 } else if (("" + board[0][2] + board[1][2] + board[2][2]).equals("xxx")) {
 iswinning = true;
 } else if (("" + board[0][0] + board[1][1] + board[2][2]).equals("xxx")) {
 iswinning = true;
 } else if (("" + board[2][0] + board[1][1] + board[0][2]).equals("xxx")) {
 iswinning = true;
 } else if (("" + board[0][0] + board[0][1] + board[0][2]).equals("xxx")) {
 iswinning = true;
 } else if (("" + board[1][0] + board[1][1] + board[1][2]).equals("xxx")) {
 iswinning = true;
 } else if (("" + board[2][0] + board[2][1] + board[2][2]).equals("xxx")) {
 iswinning = true;
 }
 return iswinning;
}
public static boolean detection1(boolean iswinning1, char[][] board) {
 if (("" + board[0][0] + board[1][0] + board[2][0]).equals("ooo")) {
 iswinning1 = true;
 } else if (("" + board[1][0] + board[1][1] + board[2][1]).equals("ooo")) {
 iswinning1 = true;
 } else if (("" + board[0][2] + board[1][2] + board[2][2]).equals("ooo")) {
 iswinning1 = true;
 } else if (("" + board[0][0] + board[1][1] + board[2][2]).equals("ooo")) {
 iswinning1 = true;
 } else if (("" + board[2][0] + board[1][1] + board[0][2]).equals("ooo")) {
 iswinning1 = true;
 } else if (("" + board[0][0] + board[0][1] + board[0][2]).equals("ooo")) {
 iswinning1 = true;
 } else if (("" + board[1][0] + board[1][1] + board[1][2]).equals("ooo")) {
 iswinning1 = true;
 } else if (("" + board[2][0] + board[2][1] + board[2][2]).equals("ooo")) {
 iswinning1 = true;
 }
 return iswinning1;
}
public static void main(String[] args) {
 Scanner keyboard = new Scanner(System.in);
 char board[][] = new char[3][3];
 int turn = 1;
 int y = 0;
 boolean validinput;
 boolean iswinning = false;
 boolean iswinning1 = false;
 while (true) {
 if (turn == 1) {
 System.out.println("player 1 it is your turn. your spot you wou"
 + "ld like to enter.\n Ex: top,botton,middle left,right,middle");
 String input = keyboard.nextLine();
 validinput = true;
 if (input.equals("top left") && board[0][0] == '\u0000') {
 board[0][0] = 'x';
 } else if (input.equals("top middle") && board[1][0] == '\u0000') {
 board[1][0] = 'x';
 } else if (input.equals("top right") && board[2][0] == '\u0000') {
 board[2][0] = 'x';
 } else if (input.equals("middle left") && board[0][1] == '\u0000') {
 board[0][1] = 'x';
 } else if (input.equals("middle middle") && board[1][1] == '\u0000') {
 board[1][1] = 'x';
 } else if (input.equals("middle right") && board[2][1] == '\u0000') {
 board[2][1] = 'x';
 } else if (input.equals("bottom left") && board[0][2] == '\u0000') {
 board[0][2] = 'x';
 } else if (input.equals("bottom middle") && board[1][2] == '\u0000') {
 board[1][2] = 'x';
 } else if (input.equals("bottom right") && board[2][2] == '\u0000') {
 board[2][2] = 'x';
 } else {
 System.out.println("You have entered an invalid space "
 + "please try again.");
 validinput = false;
 }
 if (validinput == true) {
 y++;
 turn++;
 }
 System.out.printf(" %c | %c | %c \n", board[0][0], board[1][0], board[2][0]);
 System.out.printf("____________\n");
 System.out.printf(" %c | %c | %c \n", board[0][1], board[1][1], board[2][1]);
 System.out.printf("____________\n");
 System.out.printf(" %c | %c | %c \n", board[0][2], board[1][2], board[2][2]);
 iswinning=detection(iswinning, board); 
 if (iswinning == true) {
 System.out.println("Player 1 wins!");
 System.exit(1);
 }
 if (y == 9) {
 System.out.println("the game ends in a draw.");
 System.exit(1);
 }
 }
 if (turn == 2) {
 System.out.println("player 2 it is your turn. your spot you"
 + " would like to enter.\n Ex: top,botton,middle left,right,middle");
 String input = keyboard.nextLine();
 validinput = true;
 if (input.equals("top left") && board[0][0] == '\u0000') {
 board[0][0] = 'o';
 } else if (input.equals("top middle") && board[1][0] == '\u0000') {
 board[1][0] = 'o';
 } else if (input.equals("top right") && board[2][0] == '\u0000') {
 board[2][0] = 'o';
 } else if (input.equals("middle left") && board[0][1] == '\u0000') {
 board[0][1] = 'o';
 } else if (input.equals("middle middle") && board[1][1] == '\u0000') {
 board[1][1] = 'o';
 } else if (input.equals("middle right") && board[2][1] == '\u0000') {
 board[2][1] = 'o';
 } else if (input.equals("bottom left") && board[0][2] == '\u0000') {
 board[0][2] = 'o';
 } else if (input.equals("bottom middle") && board[1][2] == '\u0000') {
 board[1][2] = 'o';
 } else if (input.equals("bottom right") && board[2][2] == '\u0000') {
 board[2][2] = 'o';
 } else {
 System.out.println("You have entered an invalid space "
 + "please try again.");
 validinput = false;
 }
 if (validinput) {
 turn--;
 y++;
 }
 System.out.printf(" %c | %c | %c \n", board[0][0], board[1][0], board[2][0]);
 System.out.printf("____________\n");
 System.out.printf(" %c | %c | %c \n", board[0][1], board[1][1], board[2][1]);
 System.out.printf("____________\n");
 System.out.printf(" %c | %c | %c \n", board[0][2], board[1][2], board[2][2]);
 iswinning1=detection(iswinning1, board); 
 if (iswinning1 == true) {
 System.out.println("Player 2 wins!");
 System.exit(1);
 }
 }
 }
}

Answer 1

Some changes you want to make are:

• Encapsulate state into classes.
• Avoid using strings for logic.
• Avoid long functions
• Don't repeat yourself.

The aim would be to get your main() looking more like this.

public static void main(String[] args) {
 Scanner keyboard = new Scanner(System.in);
 Board = new Board();
 // I'm abusing the location state to double as the
 // current user, which is a bit hacky.
 BoardLocationState turn = BoardLocationState.FILLED_X;
 while (true) {
 BoardLocation location = null;
 //Repeat until we're given a valid location
 while(location==null) {
 printPrompt(turn);
 String input = keyboard.nextLine();
 location = locationMap.get(input);
 if(board.getState(location)!=BoardLocationState.EMPTY) {
 location=null;
 }
 }
 board.setState(location, turn);
 board.printToStream(System.out);
 if(board.isWonBy(turn)) {
 printWinningMessage(turn);
 break;
 }
 if(board.isDraw()) {
 printDrawMessage(turn);
 break;
 }
 turn = nextTurn(turn);
 }
 }

A first pass at cleaning some of this up gets you something like this (very incomplete for now). See further down for what I'd be aiming to make your

public static class BoardLocation {
 public final int i;
 public final int j;
}
public static class Board {
 public enum BoardLocationState {
 FILLED_X,
 FILLED_O,
 EMPTY
 }
 // The combinations we need to check for winning or losing
 private final BoardLocation[][] rowsColumnsAndDiagonals = {
 { BoardLocation(0,0), BoardLocation(0,1), BoardLocation(0,2) },
 { BoardLocation(1,0), BoardLocation(1,1), BoardLocation(1,2) },
 { BoardLocation(2,0), BoardLocation(2,1), BoardLocation(2,2) },
 { BoardLocation(0,0), BoardLocation(1,0), BoardLocation(2,0) },
 { BoardLocation(1,0), BoardLocation(1,1), BoardLocation(2,1) },
 { BoardLocation(2,0), BoardLocation(1,2), BoardLocation(2,2) },
 { BoardLocation(0,0), BoardLocation(1,1), BoardLocation(2,2) },
 { BoardLocation(0,2), BoardLocation(1,1), BoardLocation(2,0) },
 };
 private final BoardLocationState[] state = new BoardLocationState[3*3];
 public Board() {
 for(int i=0; i<3*3; ++i) {
 state[i] = BoardLocationState.EMPTY;
 }
 }
 public BoardLocationState getState(BoardLocation location) {
 return state[location.i + location.j*3];
 }
 public void setState(BoardLocation location, BoardLocatioState newState) {
 state[location.i + location.j*3] = newState;
 }
 public boolean isWonBy(BoardLocationState user) {
 if(user==BoardLocationState.EMPTY)
 throw new RuntimeException("isWonBy should not be called with EMPTY");
 for(BoardLocation[] locations : rowsColumnsAndDiagonals) {
 if(checkRowColumnOrDiagonal(locations, user))
 return true; 
 }
 return false;
 }
 private boolean checkRowColumnOrDiagonal(BoardLocation[] locations, BoardLocationState user) {
 for(BoardLocation location : locations) {
 if(getState(location)!=user)
 return false;
 }
 return true;
 }
}

• mihaj: rowsColumnsAndDiagonals = { { BoardLocation(0,0), BoardLocation(0,1).. I think it will be better to replace for(0..2) for(0..2) and to define 2 as const, as scale-able solution.

Answer 2

Consider other computation algorithm:

O- give wage 10
x- give wage 100

to determine who wins you must sum each row and check:

if one of them is 30 - O wins
if one of them is 300 - X wins
if none - no one wins :)