Ticker Tackor Soldier Spy

This is a Tic Tac Toe game with a "new game" button. I'm after a general review of the code with some tips on how to make it simpler and tidier.

GitHub

HoboChess.java

package sample;

import javafx.event.ActionEvent;
import javafx.event.EventHandler;
import javafx.fxml.FXML;
import javafx.fxml.Initializable;
import javafx.scene.control.Button;
import javafx.scene.control.Label;
import javafx.scene.layout.GridPane;
import java.net.URL;
import java.util.ResourceBundle;

public class HoboChess implements Initializable {

    @FXML Label systemOut;
    @FXML GridPane gridPane;
    final static int ROWS = 3;
    final static int COLUMNS = 3;
    int[][] grid;
    boolean turn;

    @Override
    public void initialize(URL location, ResourceBundle resources) {
        grid = new int[COLUMNS][ROWS];
        newVisualField(ROWS, COLUMNS);
    }

    public void newGame(ActionEvent action) {
        grid = new int[COLUMNS][ROWS];
        newVisualField(ROWS, COLUMNS);
    }

    public void WhoWon(int x, int[] zone){
        if (zone[x] == 3){
            systemOut.setText("Os won!");
        }else if (zone[x] == 30){
            systemOut.setText("Xs won!");
        }
    }

    public void winCheck(){
        int[] zone = new int[COLUMNS * ROWS];
        int zoneNumber = 0;

        for (int y = 0; y () {
            @Override
            public void handle(ActionEvent e) {
                if (buttonPushed == false) {
                    if (hoboChess.turn) {
                        hoboChess.grid[y][x] = 3;
                        aButton.setText("O");
                    } else {
                        hoboChess.grid[y][x] = 30;
                        aButton.setText("X");
                    }
                    hoboChess.turn = !hoboChess.turn;
                    buttonPushed = !buttonPushed;
                    hoboChess.winCheck();
                }
            }
        });
    }
}

All your fields can be declared private. There is really no need to expose these more than necessary.

@FXML private Label systemOut;
@FXML private GridPane gridPane;
private final static int ROWS = 3;
private final static int COLUMNS = 3;
private int[][] grid;
private boolean turn;

Your initialize and newGame methods looks very much the same to me, you could change your initialize to:

public void initialize(URL location, ResourceBundle resources) {
    newGame(null);
}

public void WhoWon(int x, int[] zone){
    if (zone[x] == 3){
        systemOut.setText("Os won!");
    }else if (zone[x] == 30){
        systemOut.setText("Xs won!");
    }
}

@barq is correct that the method name can be improved, and that passing a single int value to this method should be enough, which can make it:

public void declareWinner(int value) {
    if (value == 3) {
        systemOut.setText("Os won!");
    } else if (value == 30) {
        systemOut.setText("Xs won!");
    }
}

However, these magic numbers 3 and 30 really needs to be fixed. I'd recommend an enum:

public enum Player {
    X, O;
}

And then use a Player[][] instead of int[][]. and a Player turn instead of boolean.

Your win-check approach is a bit... more complicated than necessary. You begin with:

for (int y = 0; y < 3; y++) {
    for (int x = 0; x < 3; x++) {
        zone[zoneNumber] = grid[y][x];
        zoneNumber++;
    }
}

But in reality, there is no need to transform all the positions to a number, instead you can access the 2d array directly, like this:

for (int y = 0; y < 3; y++) {
    if (zone[0][y] == zone[1][y] && zone[0][y] == zone[2][y]) {
        declareWinner(zone[0][y]);
    }
}
for (int x = 0; x < 3; x++) {
    if (zone[x][0] == zone[x][1] && zone[x][0] == zone[x][2]) {
        declareWinner(zone[x][0]);
    }
}
if (zone[0][0] == zone[1][1] && zone[0][0] == zone[2][2]) {
    declareWinner(zone[0][0]);
} else if (zone[2][0] == zone[1][1] && zone[2][0] == zone[0][2]) {
    declareWinner(zone[2][0]);
}

In ButtonCell you are doing:

hoboChess.grid[y][x] = 3;

And violà, you have flipped x and y! In other places you are doing grid[x][y] but here we have grid[y][x]. Thanks to how Tic Tac Toe works, you won't see a difference, but it is still quite important to be consistent.

Also, when changing to using an enum this can become

hoboChess.grid[x][y] = Player.X;

or

hoboChess.grid[x][y] = Player.O;

Overall it does look like you have improved over time. A little additional improvement I would recommend is to separate the game board and logic more from your Java FX controller class.

I would use one file for TicTacToeGame, containing the Player[][] array and who's turn it is, then I would use a method Player makeMove(int x, int y) that would place a mark at the specified position and return which player made the move (for the button to know what mark to set). This game logic class can be made entirely independent of your JavaFX controller, so that it can be easily used in a Java console application, a Swing application, or another framework. This is an important step and skill in programming, to be able to create re-usable classes.

StackExchange Code Review Q#80316, answer score: 7