Return to Answer

• 1

• 59.7k
• 9
• 157
• 311

Normally, X is the player to go first in Tic-Tac-Toe games. In your implementation, O goes first. This is a bit unusual.

A lot of your interfaces are using generics, but is that really necessary? I find that sometimes adding generics for these kinds of games is a case of YAGNI (You Ain't Gonna Need It). Will you ever be passing another type as the generic parameter?

You seem to rely quite a lot on String objects (in play(String move) for example). A problem with this is that you only want a very very small subset of all possible strings. It is very hard to document how the String should be formatted to be accepted. I believe that it would be a good idea to use a custom GameMove class for this, that will make the usage so much clearer.

In your FieldListener you do a bunch of mathematics with the x and y pos to retrieve the actual button that was clicked. I am not a fan of this. You're using a FieldButton class, you might as well give that a x and y property and then you can retrieve the x and y much easier.

Alternatively, you could create a specific instance of an ActionListener to it and pass x and y values along there.

mainPanel.setLayout(new GridLayout(N_OF_GUI_FIELDS, N_OF_GUI_FIELDS));
for (int i=0; i<N_OF_GUI_FIELDS*N_OF_GUI_FIELDS; ++i) {
 final FieldButton button = new FieldButton();
 button.setPreferredSize(BUTTON_SIZE);
 button.addActionListener(listener);
 buttons.add(button);
 mainPanel.add(button);
}

Here you could easily do that mathematics and call a method that creates a specific listener for that specific button, with its associated x and y values.

You might want to read some previous answers of mine about how to create multiple instances of an ActionListener that shares code.

Speaking of x and y values, I noticed that you don't really use the fact that this game is two-dimensional. Most loops that loop over all buttons is only using one variable, i. I'd recommend making more use of x and y.

Your GUI does not allow switching which AI controls which player at runtime. As you work on the AI aspect, I expect there to be more than two AIs, and the more you add the more of a mess you will have. Deal with this potential mess as soon as possible. Implementing this part in a clean way will allow players to play a bit against your Random AI, a bit against your MonteCarlo AI, and maybe even ask an AI for advice (without having them making a move). That last part is achievable in your code without too much effort as you have the GameActor return a move, instead of making the move (this is good).

boolean isX = b.getText().endsWith("X")

Not a fan of that. I'd perhaps create two instances of ActionListeners here as well.

for (final String s : "auto-X auto-O".split(" ")) {

Looping over an array of String by creating a string and splitting it is.... not ideal.

You could do:

for (final String s : new String[]{ "auto-X", "auto-O" }) {

Or you could extract a method and call it twice.

Your setStateInternal method reads the String representation of a board, and then transforms it into representations for the buttons. Why transform a 2d array to a String only to transform it back into a 2d representation again? (in the form of a value for each button). I'd skip this String entirely here and read the values directly from the game model

Your apply method would be much better named as createWorkerForActor or similar.

I recommend adhering to the Java conventions when it comes to where to place constants and fields. (At the top of the file)