HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Embarking on a Tic Tac Toe journey with JavaFX

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
toewithtictacjourneyembarkingjavafx

Problem

On my educational trek through the capabilities of JavaFX, I thought it would be a valuable challenge to create Tic Tac Toe. It took me far longer than expected to get this far, but despite likely being even further from completion, I'd like a review on the current state of things and on anything my 'blind coding' unwittingly implemented unconventionally/inefficiently/just plain weirdly.

Some names may seem strange, but that could be due to my editing the image-related code to point to URLs rather than local files for the sake of this post.

TicTacToe main class:

import javafx.application.Application;
import javafx.scene.Scene;
import javafx.scene.control.Button;
import javafx.scene.image.Image;
import javafx.scene.image.ImageView;
import javafx.scene.layout.GridPane;
import javafx.stage.Stage;

public class TicTacToe extends Application {
    static Player player = Player.O_PLAYER;

    public static void main(String[] args) {
        launch(args);
    }

    @Override
    public void start(Stage stage) {
        Button[] cells = new Button[9];
        for (int i = 0; i 
            button.setGraphic(new ImageView(retrieveMarker()))
        );
    } 
}


Player enum:

import javafx.scene.image.Image;
import javafx.scene.image.ImageView;

public enum Player {
    X_PLAYER(new ImageView(new Image("http://i.imgur.com/HY6GeUY.png"))),
    O_PLAYER(new ImageView(new Image("http://i.imgur.com/KgHHxL6.png")));

    private final ImageView view;

    Player(ImageView view) {
        this.view = view;
    }

    public Image marker() {
        return view.getImage();
    }

    @Override
    public String toString() {
        return name().charAt(0) + name().substring(2).toLowerCase();
    }
}


Follow-ups:

  • Completed Tic-Tac-Toe.



  • Tic-Tactics.

Solution

This

GridPane board = new GridPane();
for (int row = 1, col = 1, cell = 0; row <= 3; row++, col -=2) {
    board.add(cells[cell++], row, col++);
    board.add(cells[cell++], row, col++);
    board.add(cells[cell++], row, col);
}


just looks odd. The initialiszing of 3 variables , incrementing of one of then and decrementing one of them in one single for() is a little bit to much IMHO.

how about using a more obvious loop like

for (int cell = 0; cell < cells.length; cell++) {
     board.add(cells[cell], cell / 3, cell % 3);
}


ok, one has to know if he/she reads this, that this will lead to


(0 , 0 , 0)

(1 , 0 , 1)

(2 , 0 , 2)

(3 , 1 , 0)

(4 , 1 , 1)

(5 , 1 , 2)

...

but it is less code without the strange incrementing and decrementing.

The creation of the buttons should be improved. Like now you are creating a new Image for each button. Just create the Image once and reuse it like

Image image = new Image("http://i.imgur.com/KAQPbPd.png");
Button[] cells = new Button[9];
for (int i = 0; i < cells.length; i++) {
    cells[i] = new Button("", new ImageView(image);
    registerOnAction(cells[i]);
}


much better now, ins't it ?

Code Snippets

GridPane board = new GridPane();
for (int row = 1, col = 1, cell = 0; row <= 3; row++, col -=2) {
    board.add(cells[cell++], row, col++);
    board.add(cells[cell++], row, col++);
    board.add(cells[cell++], row, col);
}
for (int cell = 0; cell < cells.length; cell++) {
     board.add(cells[cell], cell / 3, cell % 3);
}
Image image = new Image("http://i.imgur.com/KAQPbPd.png");
Button[] cells = new Button[9];
for (int i = 0; i < cells.length; i++) {
    cells[i] = new Button("", new ImageView(image);
    registerOnAction(cells[i]);
}

Context

StackExchange Code Review Q#83892, answer score: 4

Revisions (0)

No revisions yet.