5
\$\begingroup\$

I made a simple tic-tac-toe AI app, where the AI, who is always O, wins or ties. I just wanted some input on it, and wanted to know if there was any ways to improve it (not the functionality of it, but how efficient it is, cleaner ways to write stuff, better naming, etc.). I also don't know if I'm supposed to go through it again and make those changes in future projects, or if I should just work on being more conscious about that stuff.

package com.example.octopoco.actualfinaltictactoeapp;

import android.os.Bundle;
import android.support.v4.app.Fragment;
import android.support.v4.app.FragmentManager;
import android.util.Log;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
import android.widget.Button;
import android.widget.TableLayout;
import android.widget.TableRow;

import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.io.PrintWriter;
import java.net.Socket;
import java.util.ArrayList;
import java.util.Random;


/**
 * Created by octopoco on 2/18/2015.
 */
public class MainFragment extends Fragment{

    private static final String DIALOG_DATE="this is whatever i want it to be";
    private String[][] board={{"-","-","-"},{"-","-","-"},{"-","-","-"}};
    private String playerCounter="X";
    private final String tag="cat";
    TableLayout tableLayout;
    ///must be reset each run
    Integer[][] placeValues=new Integer[3][3];





    ///implement onCreateMethod
    public void onCreate(Bundle savedInstanceState){
        super.onCreate(savedInstanceState);
        getActivity().setTitle("Tic Tac Toe");
    }
    ///implement onCreateView method
    public View onCreateView(LayoutInflater inflater, ViewGroup parent, Bundle savedInstanceState){
        int counter=0;
        View v=inflater.inflate(R.layout.mainfragmentlayout, parent, false);

        tableLayout=(TableLayout)v.findViewById(R.id.tablelayout1);
        ///button listener
        View.OnClickListener listener=new View.OnClickListener() {
            @Override
            public void onClick(View v) {
                switch(v.getId()){
                    case 0:

                        changeBoard(0, 0);
                        changePlayerCounter();
                        aiTurn();
                        break;

                    case 1:

                        changeBoard(0, 1);
                        changePlayerCounter();
                        aiTurn();
                        break;

                    case 2:

                        changeBoard(0, 2);
                        changePlayerCounter();
                        aiTurn();
                        break;

                    case 3:

                        changeBoard(1, 0);
                        changePlayerCounter();
                        aiTurn();
                        break;

                    case 4:

                        changeBoard(1, 1);
                        changePlayerCounter();
                        aiTurn();
                        break;

                    case 5:

                        changeBoard(1, 2);
                        changePlayerCounter();
                        aiTurn();
                        break;

                    case 6:

                        changeBoard(2, 0);
                        changePlayerCounter();
                        aiTurn();
                        break;

                    case 7:

                        changeBoard(2,1);
                        changePlayerCounter();
                        aiTurn();
                        break;

                    case 8:

                        changeBoard(2, 2);
                        changePlayerCounter();
                        aiTurn();
                        break;

                }

            }
        };
        ///set button listener for each button
        for(int i=0;i<tableLayout.getChildCount();i++){
            TableRow row=(TableRow)tableLayout.getChildAt(i);
            for(int j=0;j<row.getChildCount();j++){
                Button button=(Button)row.getChildAt(j);
                button.setText(board[i][j]);
                button.setOnClickListener(listener);
                button.setId(counter);
                counter++;

            }
        }

        return v;
    }

    public void startDialogFragment(String player){
        FragmentManager fm=getActivity().getSupportFragmentManager();
        WinDialogFragment dialog= WinDialogFragment.newInstance(player, this);
        dialog.show(fm, DIALOG_DATE);
    }

    public boolean checkIfWinning(String[][] board,String player){
        ///checks horizontal three in a row
        String winningmessage="you won player "+player;
        for(int x=0;x<3;x++){
            int counter=0;
            for(int y=0;y<3;y++){

                if(board[x][y]==player){

                    counter++;
                }
            }
            if(counter==3){

                return true;
            }
        }
        for(int x=0;x<3;x++){
            int counter=0;
            for(int y=0;y<3;y++){

                if(board[y][x]==player){
                    counter++;
                }
            }
            if(counter==3){

                return true;
            }
        }
        int counter2=0;
        int counter3=0;
        for(int xy=0;xy<3;xy++){
            if(board[xy][xy]==player){
                counter2++;
            }
            if(board[xy][2-xy]==player){
                counter3++;
            }
        }
        if(counter2==3 || counter3==3){

            return true;
        }


        return false;

    }
    ///changes board and checks winning player
    private void changeBoard(int x, int y){
        board[x][y]=playerCounter;
        printBoard();
        ///checks for winning player
        if(checkIfWinning(board,playerCounter)){
            getActivity().setTitle("PLAYER "+playerCounter+" WON");
            resetGame();

        }
        ///checks for ties
        if(true!=checkIfFilled()){
            getActivity().setTitle("DRAW");
            resetGame();
        }

    }

    ///makes display equal to board
    private void printBoard(){
        for(int i=0;i<tableLayout.getChildCount();i++) {
            TableRow row = (TableRow) tableLayout.getChildAt(i);
            for (int j = 0; j < row.getChildCount(); j++) {
                Button button = (Button) row.getChildAt(j);
                button.setText(board[i][j]);
            }
        }
    }
    private void changePlayerCounter(){
        if(playerCounter=="X"){
            playerCounter="O";


        }
        else{
            playerCounter="X";
        }
    }
    public void resetGame(){
        ///resest board
        for(int x=0;x<3;x++){
            for(int y=0;y<3;y++){
                board[x][y]="-";
            }
        }

        printBoard();

    }
    private boolean checkIfFilled(){
        for(String[] x:board){
            for(String y:x){
                if(y=="-"){
                    return true;
                }
            }
        }
        return false;
    }
    private void aiTurn() {
        //coordiantes for final move
        String player;
        int x=0;
        int y=0;
        if (playerCounter == "O") {
            player = "X";
        } else {
            player = "O";
        }

        ArrayList<Integer> bestxMoves=new ArrayList<Integer>();
        ArrayList<Integer> bestyMoves=new ArrayList<Integer>();


        String[][] fakeboard = board;
        String[][] fakeboard2;
        String debugMessage="";



        for (int i = 0; i < 3; i++) {
            for (int j = 0; j < 3; j++) {
                int pathValue2=5;
                if (fakeboard[i][j] == "-") {
                    fakeboard[i][j] ="O";
                    if (checkIfWinning(fakeboard, "O")) {
                        pathValue2 = 10;





                    }
                    else if (pathValue2 != 10) {

                        pathValue2=evaluatePath("X",fakeboard);


                    }

                    placeValues[i][j]=pathValue2;
                    fakeboard[i][j] = "-";
                }
            }
        }

        int highestValueMove=0;
        ///finds move with the highest value;
        for(int i=0;i<3;i++){
            for(int j=0;j<3;j++){
                if(board[i][j]=="-") {
                    if (placeValues[i][j] >= highestValueMove) {
                        highestValueMove = placeValues[i][j];
                    }
                }
            }
        }
        int counter=0;
        ////makes list of highest value move
        for(int i=0;i<3;i++){
            for(int j=0;j<3;j++){
                if(board[i][j]=="-") {
                    if (placeValues[i][j] == highestValueMove) {
                        bestxMoves.add(i);
                        bestyMoves.add(j);
                        counter++;
                    }
                }
            }
        }
        Random random=new Random();
        int randomBestMove;
        if(bestxMoves.size()>=1) {
             randomBestMove = random.nextInt(bestxMoves.size());
            x=bestxMoves.get(randomBestMove);
            y=bestyMoves.get(randomBestMove);
        }else{
            x=1;
            y=1;
            getActivity().setTitle(String.valueOf(counter));
        }





        changeBoard(x, y);
        changePlayerCounter();
        String bestxs="";
        for(int i:bestxMoves){
            bestxs+=String.valueOf(i);
        }
        String bestys="";
        for(int i: bestyMoves){
            bestys+=String.valueOf(i);
        }
        for(int i=0;i<3;i++){
            for(int j=0;j<3;j++){
                placeValues[i][j]=5;
            }
        }
        Log.d(tag, bestxs+"    "+bestys+"    "+highestValueMove+"     "+debugMessage);
    }
    //for testing purpose only. may delete later

    private int evaluatePath(String Player, String[][] fakeboard) {

        switch (Player) {
            case "X":
                int returnValue = 10;
                boolean checkIfLoopRan=false;
                for (int i = 0; i < 3; i++) {
                    for (int j = 0; j < 3; j++) {
                        int pathValue = 5;
                        if (fakeboard[i][j] == "-") {
                            checkIfLoopRan=true;
                            fakeboard[i][j] = "X";
                            if (checkIfWinning(fakeboard, "X")) {
                                pathValue = 0;


                            } else if (pathValue != 0) {

                                ///what does pathValue even do!?!?!
                                pathValue = evaluatePath("O",fakeboard);



                            }
                            if (pathValue < returnValue) {
                                returnValue = pathValue;
                            }
                            fakeboard[i][j] = "-";
                        }







                    }
                }
                if(!checkIfLoopRan){
                    returnValue=5;
                }
                return returnValue;



            case "O":
                int returnValue2 = 0;
                boolean checkIfLoopRan2=false;
                for (int i = 0; i < 3; i++) {
                    for (int j = 0; j < 3; j++) {
                        int pathValue = 5;
                        if (fakeboard[i][j] == "-") {
                            checkIfLoopRan2=true;
                            fakeboard[i][j] = "O";
                            if (checkIfWinning(fakeboard, "O")) {


                                pathValue = 10;


                            } else if (pathValue != 10) {



                                pathValue=evaluatePath("X",fakeboard);


                            }
                            if (pathValue > returnValue2) {
                                returnValue2 = pathValue;
                            }

                            fakeboard[i][j] = "-";
                        }



                    }
                }
                if(!checkIfLoopRan2){
                    returnValue2=5;
                }
                return returnValue2;

        }
        return 5;


    }
    public void returnMoveValues(){

    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ Welcome to Code Review! Good job on your first question! \$\endgroup\$ – SirPython Nov 21 '15 at 0:19
  • 1
    \$\begingroup\$ Thanks SirPython! Glad i posted the question to the right place. \$\endgroup\$ – averythoughtfulguy Nov 21 '15 at 19:34
1
\$\begingroup\$

Indicate the currentPlayer as an integer, so you can switch the active player more easily.

private int currentPlayer = 0;
private static final String[] PLAYER_CHAR = {"X", "O"}; // player 0 plays with X, player 1 plays with O

private void changeCurrentPlayer() {
    if(++currentPlayer == 2){
        currentPlayer=0;
    }
}

You can represent the board as two integers. Where a bit is set to 1 when a player occupies it. This reduces a lot of switch/for logic. And it's much faster as well.

private int[] playerMask = {0, 0}; // both initially haven't set anything
private static final String EMPTY_CHAR = "-";
private static final int ROW_MASK = 7,      // 000000111
                            COL_MASK = 73,      // 001001001
                            DIAG1_MASK = 273,   // 100010001
                            DIAG2_MASK = 84,    // 001010100
                            FULL_BOARD = 511;   // 111111111

public void resetGame() {
    /// simply resest board by setting values to 0
    playerMask[0]=playerMask[1]=0;
    printBoard();
}

// check if all fields are taken by the players
private boolean checkIfFilled() {
    return (playerMask[0]|playerMask[1])==FULL_BOARD;
}

public boolean checkIfWinning(){
    // checks horizontal three in a row
    for(int x=0;x < 3;x++){
        int rowMask = (ROW_MASK << (x*3));
        if((rowMask & playerMask[currentPlayer]) == rowMask){
            return true;
        }
    }
    // checks vertical three in a row
    for(int x=0;x < 3;x++){
        int colMask = (COL_MASK << x);
        if((colMask & playerMask[currentPlayer]) == colMask){
            return true;
        }
    }
    // check diagonals
    if((DIAG1_MASK & playerMask[currentPlayer]) == DIAG1_MASK){
        return true;
    }
    if((DIAG2_MASK & playerMask[currentPlayer]) == DIAG2_MASK){
        return true;
    }
    return false;
}

And printing it, can just be calculated instead of being saved all the time

/// makes display equal to board
private void printBoard(){
    for(int i=0,id=0;i<tableLayout.getChildCount();i++) {
        TableRow row = (TableRow) tableLayout.getChildAt(i);
        for (int j = 0; j < row.getChildCount(); j++) {
            Button button = (Button) row.getChildAt(j);                
            // Determine text
            button.setText(getCurrentDisplayCharForId(id++));
        }
    }
}

// If taken by player 0 => 'X', by player 1 => 'O', else '-' 
private String getCurrentDisplayCharForId(int id){
    if((playerMask[0]&(1<<id)) > 0){
        return PLAYER_CHAR[0];
    }else if((playerMask[1]&(1<<id)) > 0){
        return PLAYER_CHAR[1];
    }
    return EMPTY_CHAR;
}

At last the AI part. I didn't check the correctness of it, but that's because it's hard to read. Using our bitmasks this can be done recursive (MINIMAX).

private static final Random random = new Random();
private void aiTurn() {
    int bestId = -1;
    int outcome = -2;
    int res;
    for(int id=0;id<9;id++){
        if((playerMask[0]&(1<<id)) > 0) continue; // already set by player 0
        if((playerMask[1]&(1<<id)) > 0) continue; // already set by player 1
        playerMask[currentPlayer] |= (1<<id);
        if(checkIfWinning()){
            res = 1;
        }else{
            changeCurrentPlayer();
            res = evaluatePath();
            changeCurrentPlayer();
        }
        if(outcome < res || (outcome == res && random.nextBoolean())){
            outcome = res;
            bestId = id;
        }
        playerMask[currentPlayer] -= (1<<id);
    }
    changeBoard(bestId);
}
// -1 => lose
// 0 => draw
// 1 => win
private int evaluatePath() {
    if(!checkIfFilled()){
        return 0;
    }
    int best = -2;
    for(int id=0;id<9;id++){
        if((playerMask[0]&(1<<id)) > 0) continue; // already set by player 0
        if((playerMask[1]&(1<<id)) > 0) continue; // already set by player 1
        playerMask[currentPlayer] |= (1<<id);
        if(checkIfWinning()){
            return 1;
        }
        changeCurrentPlayer();
        best = Math.max(best, evaluatePath()*-1);
        changeCurrentPlayer();
        playerMask[currentPlayer] -= (1<<id);
    }
    return best;
}

For completeness, I may have changed other methodes as well:

public void onClick(View v) {
    int id = v.getId();
    if(id == 0){
        return;
    }
    changeBoard(id);
    aiTurn();
}

/// changes board and checks winning player
private void changeBoard(int id){
    playerMask[currentPlayer] |= (1<<id); // Current player takes id
    printBoard();
    ///checks for winning player
    if(checkIfWinning()){
        getActivity().setTitle("PLAYER "+PLAYER_CHAR[currentPlayer]+" WON");
        resetGame();

    }
    ///checks for ties
    if(!checkIfFilled()){
        getActivity().setTitle("DRAW");
        resetGame();
    }
    changeCurrentPlayer();
}

On demand UPDATE (bitwise operations)

Also see these examples

Our board consists out of 9 possible positions:

012
345
678

So we only need to work with the first 9 bits of an integer: 876543210

To check if winning we can use the following masks:

  • ROW: (000000111)bin = (7)dec (= representation of the first row)

To check all 3 rows, we can shift it per 3 places to the left.

((000000111)bin << 3*0) = (000000111)bin
((000000111)bin << 3*1) = (000111000)bin
((000000111)bin << 3*2) = (111000000)bin
  • COLUMN: (001001001)bin = (73)dec (= representation of the first col)

To check all 3 columns, we can shift it per 1 place to the left.

((001001001)bin << 0) = (001001001)bin
((001001001)bin << 1) = (010010010)bin
((001001001)bin << 2) = (100100100)bin
  • DIAGONALS:

2 diagonals masks:

(100010001)bin = (273)bin
(001010100)bin = (84)bin

All that is left to is checking if all bits in the mask are set to true by the current player. This is very easy using the bitwise &-operator. Example:

(000000111)bin & (100100110)bin = (000000110)bin

if((mask & playerBits) == mask) => All bits in mask are set to true

REMARK: Instead of looping, you could hardcode all row and column masks. Which are still only 8 masks.

If you understand this, you should also be able to understand how to

  • check if a board is full
  • check which player occupies which place
  • set a bit for the current player
\$\endgroup\$
  • \$\begingroup\$ I'm kind of new to java, and am not very experienced with bits. Do you think you could explain some of the bit-wise operations, and how they work for its application? \$\endgroup\$ – averythoughtfulguy Dec 6 '15 at 21:49
  • \$\begingroup\$ There you go, hope this helps \$\endgroup\$ – Sam Segers Dec 7 '15 at 14:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.