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

Tic-Tac-Toe game simulator in Java

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

Problem

I've completed this Tic Tac Toe simulator in java and so far it's working. It's just supposed to generate a pre-played game and announce the winner. I'm required to use a 2D array and this is the way I've found to get it done. But I ask more experienced developers if there is a way to do this that is a little less convoluted. Or for any pointers you may have.

PS: the display and winner label are in the rest of my code but that's not the main part I'm worried about. the int 1 stands for O and the int 2 stands for X.

```
public class gameBord {

// tic tac toe bord
public int[][] tBord = new int[3][3];
public int counter; // to count through the 9 spaces
public int x; // to hold the index of x
public int y; // to hold the index of y
public int xs; // to hold how many current x marks there are on the board
public int os; // to hold # of y marks on bord (these are to make sure there aren't
// more marks than the player has turns.

public int xAndO; // holds the temp variable to indicate which letter will
// be stored in the current index on the array

public Random r; // a random int between 1 and 2 will be chosen to define x and o

public String winMessage; // string to be displayed on lable to display winner

public void game(){

// 9 for the 9 squares in tic tac toe
counter = 9;
// x and y stand for the indexs in the array
x = 0;
y = 0;
os = 0;
xs = 0;

System.out.println("clicked");

// these while loops will loop through the rows this one being the last row
while (counter > 0){

// before the last row we go through the second row
while (counter > 3){

// before the second row we go through the first row (first 3 colmns)
while (counter > 6){

// label which row you're in
System.out.println("Row 1");

// get a random numb

Solution

Code structure

The nested while loops obscure the structure of your code. In method game(), all iterations of middle loop are being performed during the first iteration of the outer loop, before the rest of its body, and all iterations of the innermost loop are performed during the first iteration of the middle loop, before the rest of that loop's body. That's effectively three separate, unnested loops running in sequence, and it would be much clearer to write it that way.

Variable scope and protection

Class gameBord has multiple instance variables that should instead be local variables of method game(). In fact, the only ones that appear to make sense as instance variables are tBord and winMessage, and possibly also r. The others have no business being part of the state of a gameBord; they are details of a particular run of method game(), and therfore should be local variables of that method.

Furthermore, few, if any, of the variables that are retained at class level should be public. As a matter of style, convention, and good practice, member variables should normally be private, with accessor methods where appropriate. There are exceptions, but you've presented no reason for me to think that any of those are in play here.

Improper use of Random

You initialize a new instance of Random for each random number you want to generate. This is at best wasteful, but it may actually be biting you by producing results much less uniformly distributed than you presumably hope to get. The correct approach is to instantiate Random once, and then to draw all (pseudo)random numbers you need from that one instance.

Convoluted algorithm

You're just filling the board, not simulating a game move-by-move. With that being the case, there are far simpler ways to go about the task than the one you have implemented. For example,

  • assume player 1 goes first, so that the final board will contain five 1s and four 2s;



  • create a nine-element List of Integers with that many 1s and 2s in any order (does not need to be random; Collections.nCopies() could help here);



  • shuffle the List via Collections.shuffle();



  • read out the List into the board array via a simple loop.



That can be implemented in about 8 lines, as opposed to your 100-ish, with those 8 being largely self-documenting. Or just a few more if you want also to output the per-position messages that your current code does.

Impossible / incomplete results

Your simple approach to filling the board affords results that could never arise in a real game. The example run you presented in fact demonstrates this: players 1 and 2 both have threes-in-a-row. This may not be a concern in practice (i.e. that may be what you intend to do).

You also do not produce representations of many of the possible games -- specifically, those that end in fewer than 9 moves. This, too, might not be a concern in practice.

Spelling

You're at least consistent, but in English, a flat surface for playing a game upon is a "board", not a "bord". The misspelling makes no functional difference, but it is distracting.

Code style

The code style is pretty good in general, but the most widely accepted coding conventions call for class names to start with an initial capital letter: GameBoard.

Context

StackExchange Code Review Q#145321, answer score: 4

Revisions (0)

No revisions yet.